Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code quality: reimplement DbChange as variant #35

Closed
msm-code opened this issue Apr 11, 2020 · 1 comment
Closed

Code quality: reimplement DbChange as variant #35

msm-code opened this issue Apr 11, 2020 · 1 comment
Labels
good first issue Good for newcomers level:medium This issue is of average difficulty priority:low Priority: low status:up for grabs This issue is a good candidate for PR type:refactoring Improving the code quality

Comments

@msm-code
Copy link
Contributor

DbChange class is clearly used in ways it was not designed for.

The class definition:

enum class DbChangeType {
    Insert = 1,
    Drop = 2,
    Reload = 3,
    ToggleTaint = 4,
    NewIterator = 5,
    UpdateIterator = 6
};

class DBChange {
   public:
    DbChangeType type;
    std::string obj_name;
    std::string parameter;

    DBChange(const DbChangeType &type, const std::string &obj_name,
             const std::string &parameter = "")
        : type(type), obj_name(obj_name), parameter(parameter) {}
};

Uses parameter (a public field!) as a black-box container for basically everything, and database is expected to dispatch this checking change type.

The most egregious example of this is the UpdateIterator change, which uses string parameter to pass two integers (!).

I think we should rewrite this class using std::variant, similarly to the Command class.

So we'll have use:

using DbChange =
    std::variant<InsertDatasetChange, DropDatasetChange, ......>;

And dispatch using std::visit method.

@msm-code msm-code added good first issue Good for newcomers level:medium This issue is of average difficulty priority:low Priority: low status:up for grabs This issue is a good candidate for PR type:refactoring Improving the code quality labels Apr 11, 2020
@msm-code
Copy link
Contributor Author

That's a nice refactoring, but since the project is, let's say, resource starved, we probably don't have time for that.

It's still something I'd like to see, so if anyone reads this, feel free to reopen if you want to work on this. I probably won't, and this is not critical for anything, so closing this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers level:medium This issue is of average difficulty priority:low Priority: low status:up for grabs This issue is a good candidate for PR type:refactoring Improving the code quality
Projects
None yet
Development

No branches or pull requests

1 participant