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

Exercise code/operators, about the solution #312

Open
chavid opened this issue Oct 6, 2022 · 5 comments
Open

Exercise code/operators, about the solution #312

chavid opened this issue Oct 6, 2022 · 5 comments
Labels

Comments

@chavid
Copy link
Contributor

chavid commented Oct 6, 2022

@sponce @bernhardmgruber @hageboeck I generally recommand to make any unary constructor explicitly. Yet, in this example, how dangerous is it to let the compiler transform an int into some Fraction ? If you allow this, the implementation of operators is made largely simpler.

@bernhardmgruber
Copy link
Contributor

I would still advice against making the unary constructor explicit, since we want to also set good examples.

Apart from that, looking at the solution to the operators exercise, where do you think an implicit conversion from int to Fraction would help?

@chavid
Copy link
Contributor Author

chavid commented Oct 6, 2022

If you allow the implicit conversion, for each operator, on eonly need to implement the version (Fraction const &, Fraction const &), and there is no need to implement (Fraction const &, int) and (int, Fraction const &).

@bernhardmgruber
Copy link
Contributor

If you allow the implicit conversion, for each operator, on eonly need to implement the version (Fraction const &, Fraction const &), and there is no need to implement (Fraction const &, int) and (int, Fraction const &).

Hmm. I see. That would save you 5 operators, indeed. However, it might be less efficient since you now perform extra work in operator+= and operator*=. But apart from that, I think nothing forbids you from making your constructor non-explicit. It is a valid solution and will pass the tests in main.

@chavid
Copy link
Contributor Author

chavid commented Oct 6, 2022

Also, if performance is the aim, I feel that implementing >= <= > reusing > and == is not always optimal ;) To be checked, I did not dig this. And I rather prefer the spirit of the exercise the way it is currently, promoting "consistency first", which you ensure implementing >= <= > from > and ==.

@stale
Copy link

stale bot commented Oct 6, 2023

This issue or pull request has been automatically marked as stale because it has not had recent activity. Please manually close it, if it is no longer relevant, or ask for help or support to help getting it unstuck. Let me bring this to the attention of @klieret @wdconinc @michmx for now.

@stale stale bot added the stale label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants