exercism / cpp

Exercism exercises in C++.
https://exercism.org/tracks/cpp
MIT License
252 stars 205 forks source link

feat(leap): add approaches #759

Closed vaeng closed 6 months ago

vaeng commented 6 months ago

closes #758

@BethanyG I would appreciate some language nitpicks :)

siebenschlaefer commented 6 months ago

Oh, I forgot: Three Four things that come up often in my mentoring sessions:

  1. Omitting the name of the parameter in the declaration. I tell them:

    But a user of your function does not only need to know the type of the parameter (int) but also its meaning (it's a year). I'd suggest giving that parameter a name in the declaration. That makes no difference to the compiler, but it helps humans who read that declaration. It's a cheap way of documenting that aspect of a function.

  2. Using an unsigned type for the parameter. I tell them:

    I guess you did that because you think the year will always be positive, right? ES.100-102 of the C++ Core Guidelines talk about signed and unsigned types. They basically state that you should use signed types for arithmetic, unsigned types for bit manipulation, and that you should never mix them. Even if you use an unsigned type for the argument the user could still pass a negative integer to the function and that's a perfectly valid thing to do because the conversion from signed to unsigned is well defined. But the result of the function would probably be unexpected. Some might argue that negative years or the year 0 do not exist, or that the Gregorian Calendar wasn't invented back then. The Roman Catholic Church made the Gregorian Calendar official in 1582, starting with October, 15th. Is 1580 a leap year? If 1580 is a leap year, why not the year 0 or -444? The adoption of the calendar took many years, Saudi Arabia only started using it in 2016. Was 2012 a leap year or does it depend on the location? If you want to have some "start year" and consider all years before that not to be leap years you should at least document that. But IMHO a better alternative is to implement is_leap_year() without a start year. Extending the Gregorian calendar backwards to dates before Oct 15th, 1582, even with the year 0 and negative years, gives you the Proleptic Gregorian calendar. That calendar is widely used, e.g. by ISO 8601.

  3. Using a "result variable" like this:

    bool result = false;
    if (year % 4 == 0) {
        result = true;
        if (year % 100 == 0) {
            result = false;
            if (year % 400 == 0) {
                result = true;
            }
        }
    }
    return result;

    I always try motivating them to find a solution that returns directly, with multiple return statements or (even better) to find a single expression ... and then choose what they like more.

  4. Often students order the sub-expressions slightly differently, e.g. like this:

    return year % 400 == 0 || (year % 100 != 0 && year % 4 == 0);

    When I get the expression that this was an easy exercise for them I challenge them. I tell them:

    This solution performs 2.9875 (or some other number, depending on their solution) modulo operations on average. Can you find a way to reorder the three sub-expressions year % 400 == 0, year % 4 == 0, and year % 100 != 0 in a way that the function performs the fewest operations on average?

    and when they've found the that optimal solutions or when they give up I point them to webpage of Lord chrono:

    Howard Hinnant wrote a short paragraph about the order of operations on his web page about date-related algorithms.

I know that not all of this information will be usable in the "approaches", but maybe at least some other mentor might find it useful.

vaeng commented 6 months ago

I know that not all of this information will be usable in the "approaches", but maybe at least some other mentor might find it useful.

This is very good information. As many of those are versions that would evolve into one of the provided approaches, I would not include them as their own approaches.

Do we have mentoring notes on the track were this could be used? I have not done anything with the mentoring system yet.

vaeng commented 6 months ago

I will merge this to use it for the article, we could add more in the future.