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

feat(leap): add approaches #759

Merged
merged 2 commits into from
Jan 10, 2024
Merged

feat(leap): add approaches #759

merged 2 commits into from
Jan 10, 2024

Conversation

vaeng
Copy link
Contributor

@vaeng vaeng commented Jan 7, 2024

closes #758

@BethanyG
I would appreciate some language nitpicks :)

@vaeng vaeng added the x:type/content Work on content (e.g. exercises, concepts) label Jan 7, 2024
@vaeng vaeng self-assigned this Jan 7, 2024
Copy link
Contributor

@siebenschlaefer siebenschlaefer left a comment

Choose a reason for hiding this comment

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

You put a lot of effort in. I like it.

I've mentored this exercise a lot. There are some more notable approaches:

  1. Some students like to create three variables for the three atomic sub-expressions:
    bool div_by_4 = year % 4 == 0;
    bool div_by_100 = year % 100 == 0;
    bool div_by_400 = year % 400 == 0;
    return div_by_4 && (!div_by_100 || div_by_400);
    IMHO that's not bad, it makes the final return easy to read and the overhead is probably acceptable in most cases.
  2. A few students did use a lambda:
    auto div_by = [year](int divisor) { return year % divisior == 0; };
    return div_by(4) && (!div_by(100) || div_by(400));
    That's similar to (1) but avoids the computational overhead.
  3. A lot of students want to solve this exercise step by step. Either starting with the most narrow case:
    if (year % 400 == 0)
        return true;
    if (year % 100 == 0)
        return false;
    if (year % 4 == 0)
        return true;
    return false;
    Or starting with the most general case:
    if (year % 4 != 0)
        return false;
    if (year % 100 != 0)
        return true;
    if (year % 400 == 0)
        return false;
    return true;
    That's definitely not wrong and it's easy to read. I usually challenge them to find a one-liner to get a shorter and more concise solution ... and then choose what they like more.
  4. The two approaches from (3) can also be written as a series of nested if statements.
    if (year % 4 == 0)
    {
        if (year % 100 == 0)
        {
            if (year % 400 == 0)
                return true;
            return false
        }
        return true;
    }
    return false;

Whether you want to add or mention some of them in your approaches is of course up to you.


All lines produce the same result, but differ a lot in readability.
The reason for their equal results, is the order of operator evaluation (`%`, `!=`, `||`, etc) or [precedence][operator-precedence].
In this example the first operator to get the attention of the compiler is the remainder, followed by the equality operators.
Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds like the three "a % b == 0" sub-expressions would be evaluated first.
Sadly I don't have a better alternative yet, perhaps Bethany ...?

Copy link
Contributor Author

@vaeng vaeng Jan 8, 2024

Choose a reason for hiding this comment

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

I could be more explicit an write down every line as they are evaluated. But that might be a bit too much.

exercises/practice/leap/.approaches/introduction.md Outdated Show resolved Hide resolved
exercises/practice/leap/.approaches/introduction.md Outdated Show resolved Hide resolved
@siebenschlaefer
Copy link
Contributor

siebenschlaefer commented Jan 7, 2024

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
Copy link
Contributor Author

vaeng commented Jan 9, 2024

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
Copy link
Contributor Author

vaeng commented Jan 10, 2024

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

@vaeng vaeng merged commit b6b3993 into main Jan 10, 2024
7 checks passed
@vaeng vaeng deleted the feat(leap)--add-approaches-to-leap branch January 10, 2024 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:type/content Work on content (e.g. exercises, concepts)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add approaches for leap exercise
2 participants