[Code Review] Fraction Class
(Disclaimer Note)
I thank you very much for taking the time to go through this. Please accept my apologies for the long post. English is not my first language and I wanted to be as clear as possible for the review.
(End of Disclaimer)
Hello there. For some context, I bought a book called "The Programmer Idea Book", so that I can practice and better my C++ skills with the projects it brings. One such a project is making a Fraction Class which I have written. The Class can add, subtract, multiply and divide. Simplification needs to be implemented and I may do that later.
At first I had all the code in a single file, but after writing the whole thing I decided to move Fraction to its own class instead (For organizational purposes).
A few points that I considered when writing this class.
-Initially I was considering implementing operator overload for the math, but when I was researching on the subject I found this.
https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading/4421708#4421708
My take away from that article and some other youtube videos is that although Operator Overloads are useful and have their place, one should try to avoid them as much as possible. However when I was coding the class I also consulted "The C++ Programming Language 4th Edition" by Bjarne Stroustrup, in there he goes with the Complex Number example and explains the reasoning behind Operator Overload which actually made sense and was applicable for my Fraction Class as well. Yet, I still have my doubts as to why should I or shouldn't I use overload and when.
For the most part you can ignore main.cpp as it is still in progress, I want to implement an input function for menu selection as well as for entering the fractions when performing operations. The Focus has to be mostly in Fraction.h and Fraction.cpp
Another point I would like you to consider is that overloads of +=, -=, *= and /= are class members while +, -, * and / are helpers that I wrote outside the class in the header files. My initial approach was to put all the overloads as class members, but according to the book this is not necessary as the first set of overload will modify the class itself, while the second set will not. (To be honest I am still trying to get my head around it.) I tried to put the helper functions in Fraction.cpp, but I could not find a way to call them from main, upon some research I was made aware that helper functions are better off outside the class, but on the header file and while this makes sense I don't understand why that is. A final clarification I would like to get is the use of inline. After writing my helper functions in the header file and outside the class, I got an error saying that operator+ was already defined somewhere else in the code, but I couldn't find where, so I wrote the functions as inline and then it worked. The specific question here is, what would cause the compiler to give me that already defined error and why does inline fixes the problem?
If you have any suggestions or comments for the code, I would love to hear them. I am always looking for feedback on how to better my coding skills.
To keep this as short as possible I am posting a link to the github project.
https://github.com/alienguard140/IdeasBookPractice/tree/master/FractionOperations/FractionOperations
I thank you very much for taking the time to go through this. Please accept my apologies for the long post. English is not my first language and I wanted to be as clear as possible for the review.
(End of Disclaimer)
Hello there. For some context, I bought a book called "The Programmer Idea Book", so that I can practice and better my C++ skills with the projects it brings. One such a project is making a Fraction Class which I have written. The Class can add, subtract, multiply and divide. Simplification needs to be implemented and I may do that later.
At first I had all the code in a single file, but after writing the whole thing I decided to move Fraction to its own class instead (For organizational purposes).
A few points that I considered when writing this class.
-Initially I was considering implementing operator overload for the math, but when I was researching on the subject I found this.
https://stackoverflow.com/questions/4421706/what-are-the-basic-rules-and-idioms-for-operator-overloading/4421708#4421708
My take away from that article and some other youtube videos is that although Operator Overloads are useful and have their place, one should try to avoid them as much as possible. However when I was coding the class I also consulted "The C++ Programming Language 4th Edition" by Bjarne Stroustrup, in there he goes with the Complex Number example and explains the reasoning behind Operator Overload which actually made sense and was applicable for my Fraction Class as well. Yet, I still have my doubts as to why should I or shouldn't I use overload and when.
For the most part you can ignore main.cpp as it is still in progress, I want to implement an input function for menu selection as well as for entering the fractions when performing operations. The Focus has to be mostly in Fraction.h and Fraction.cpp
Another point I would like you to consider is that overloads of +=, -=, *= and /= are class members while +, -, * and / are helpers that I wrote outside the class in the header files. My initial approach was to put all the overloads as class members, but according to the book this is not necessary as the first set of overload will modify the class itself, while the second set will not. (To be honest I am still trying to get my head around it.) I tried to put the helper functions in Fraction.cpp, but I could not find a way to call them from main, upon some research I was made aware that helper functions are better off outside the class, but on the header file and while this makes sense I don't understand why that is. A final clarification I would like to get is the use of inline. After writing my helper functions in the header file and outside the class, I got an error saying that operator+ was already defined somewhere else in the code, but I couldn't find where, so I wrote the functions as inline and then it worked. The specific question here is, what would cause the compiler to give me that already defined error and why does inline fixes the problem?
If you have any suggestions or comments for the code, I would love to hear them. I am always looking for feedback on how to better my coding skills.
To keep this as short as possible I am posting a link to the github project.
https://github.com/alienguard140/IdeasBookPractice/tree/master/FractionOperations/FractionOperations
The functions in main() seem to have a lot of code duplication, perhaps another function or two should be considered.
Also your += and -= overloads look incorrect. If the fractions have the same denominator shouldn't the denominator stay the same?
Also shouldn't the print() function be flagged as const? Have you considered overloading the insertion operator instead of even having a "print()" function?
Also since you're not really using the destructor get rid of it. And the no arg constructor should probably be defined as default in the class definition to let the compiler generate that constructor.
Also your += and -= overloads look incorrect. If the fractions have the same denominator shouldn't the denominator stay the same?
Also shouldn't the print() function be flagged as const? Have you considered overloading the insertion operator instead of even having a "print()" function?
Also since you're not really using the destructor get rid of it. And the no arg constructor should probably be defined as default in the class definition to let the compiler generate that constructor.
Last edited on
| A final clarification I would like to get is the use of inline. After writing my helper functions in the header file and outside the class, I got an error saying that operator+ was already defined somewhere else in the code, but I couldn't find where, so I wrote the functions as inline and then it worked. The specific question here is, what would cause the compiler to give me that already defined error and why does inline fixes the problem? |
Well normally you shouldn't have executable code in a header, the two major exceptions are inline functions and template functions.
When you have normal function implementations inside a header those functions will get copied into each function that #includes that header which causes the problems shown above.
Hi, thank you for your response.
I noted the code duplication you talked about in main, I am working on refactoring that code as I write this. Will probably have it updated in 2 days tops.
Yes += and -= are coded incorrectly. I don't even know what I was thinking. I feel silly right now... (As in I have brought shame to myself and my family).
I am working on overloading both the insertion and the output operators. Trying to figure out the best way to do so with the insertion operator as I don't fully understand the whole mechanics behind overloading yet.
Thank you for your help, I wouldn't be working on these issues had you not pointed them out. As I said, this code will be updated in the next day or two with the suggested fixes...
I noted the code duplication you talked about in main, I am working on refactoring that code as I write this. Will probably have it updated in 2 days tops.
Yes += and -= are coded incorrectly. I don't even know what I was thinking. I feel silly right now... (As in I have brought shame to myself and my family).
I am working on overloading both the insertion and the output operators. Trying to figure out the best way to do so with the insertion operator as I don't fully understand the whole mechanics behind overloading yet.
Thank you for your help, I wouldn't be working on these issues had you not pointed them out. As I said, this code will be updated in the next day or two with the suggested fixes...
Topic archived. No new replies allowed.