Skip to content

Simplify the two functions that handle or and and operators #147

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

Merged

Conversation

NlightNFotis
Copy link
Contributor

Instead of having two very similar functions that do the same thing but differ in one line, provide one polymorphic function that calls the appropriate function through a function pointer passed to it. I think this should simplify maintenance in the future.

Also add some documentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine but I'd like someone else's perspective on how comfortable they are with using higher order functions.

-- --
-- int intA = (int) A; // 1
-- int intB = (int) B; // 1
-- int intC = A | B // bitwise `or` or `and`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just say bitop instead of | followed by // bitop can be bitor, bitand

and theoretically also xor and things like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the actual generated code looks more like (bool) ((int) A bitop (int) B)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, indeed, I wanted to make it as simple as possible, and try to simulate the sequence of the calls happening inside the function.

Copy link
Contributor

@xbauch xbauch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@NlightNFotis NlightNFotis force-pushed the simplify_operator_handling branch from fe81ccb to 58d8907 Compare March 6, 2019 17:07
And provide some documentation for them.
@NlightNFotis NlightNFotis force-pushed the simplify_operator_handling branch from 58d8907 to 4ca60b7 Compare March 6, 2019 17:08
@NlightNFotis NlightNFotis merged commit 72463ae into diffblue:master Mar 6, 2019
@NlightNFotis NlightNFotis deleted the simplify_operator_handling branch March 6, 2019 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants