Search code examples
c++standards

What to consider when looking at multiple methods to achieve the same result while coding?


I am currently coding in C++, creating an all rounded calculator that, when finished, will be capable of handling all major and common mathematical procedures.

The current wall I am hitting is from the fact I am still learning about to profession we call being a programmer.

I have several ways of achieving a single result. I am curious as to whether I should pick the method that has a clear breakdown of how it got to that point in the code; or the method that is much shorter - while not sacrificing any of the redability.

Below I have posted snippets from my class showing what I mean.

This function uses if statements to determine whether or not a common denominator is even needed, but is several lines long.

Fraction Fraction::addFraction(Fraction &AddInput)
{
    Fraction output;

    if (m_denominator != AddInput.m_denominator)
    {
        getCommonDenominator(AddInput);

        output.setWhole(m_whole + AddInput.m_whole);
        output.setNumerator((m_numerator * firstchange) + (AddInput.m_numerator * secondchange));
        output.setDenominator(commondenominator);
    }
    else
    {
        output.setWhole(m_whole + AddInput.m_whole);
        output.setNumerator(m_numerator + AddInput.m_numerator);
        output.setDenominator(m_denominator);
    }

    output.simplify();

    return output;
}

This function below, gets a common denominator; repeats the steps on the numerators; then simplifies to the lowest terms.

Fraction Fraction::addFraction(Fraction &AddInput)
{
    getCommonDenominator(AddInput);

    Fraction output(m_whole + AddInput.m_whole, (m_numerator * firstchange) + (AddInput.m_numerator * secondchange), commondenominator);

    output.simplify();

    return output;
}

Both functions have been tested and always return the accurate result. When it comes to coding standards... do we pick longer and asy to follow? or shorter and easy to understand?


Solution

  • Your first priority with your code should be that it's correct.
    Your second priority with code should be "If someone who's never seen this before is going to make a tiny change, which one is he less likely to break?

    There's actually a lot that goes into this. How difficult is it to understand at a high level? How abstracted out are arcane details? Are there any surprises? What quirks do you have to know about? Are there edge cases that have to be handled?

    The reasons that this second priority is important are:

    • it's key to preventing you from writing bugs in the first place
    • it's easier to find bugs later
    • it's easier to fix bugs later
    • despite whatever you think, you won't remember the details in 6 months.

    Both implementations appear about equally difficult in complexity per branch, but the first one has branches, so I'd lean toward the second for understandability. Details seem abstracted out in both, and if there's surprises or quirks, I don't immediately see them (but that's sort of the point, that they can be easily overlooked). I don't see any special handling for edge cases, so if edge cases exist in either, comments would be good.

    Unrelated to picking, but while on the topic of reviewing code, It's unclear how either handles fractions that have no fractional part, but that might be part of the full class documentation, which would be fine. Both codepaths take AddArgument by mutable reference, which is bad, and require this to be mutable as well, which is also bad. Both have methods named get*() that appear to modify (getCommonDenominator), which is bad. The code appears to be using variables that are external (firstchange? secondchange?) which is a major strike against preventing bugs.