Mentor

Lines 12-14 could as easily be self.students = sorted(students or Garden.DEFAULT_CHILDREN_NAMES)

In my first iteration, Lines 10-14 looked like:

def __init__(self, diagram, students=None):
    if students is not None:
        self.students = sorted(students)
    else:
        self.students = Garden.DEFAULT_CHILDREN_NAMES

Me

Isn't that considered a hack, is there a link to the specification of what this code does? I may be a bit old school but in my head, by default I'll always assume OR operator returns a boolean. I looked for it and I'm not the only one being confused. On the other hand I kind of like having that done in a single line (as this is an operation that is used a lot).

I searched for a cleaner (to my non-python expert eyes), what do you feel about this:

sorted(students if students is not None else Garden.DEFAULT_CHILDREN_NAMES)

Or even a bit more hacky version (I don't like too much relying on the fact that None == False either):

sorted(students if students else Garden.DEFAULT_CHILDREN_NAMES)

I have to admit that I could use this solution and get over it, just because I would be lazzy to type is not None over and over.

Mentor

There's no hack, Python's and has always returned either the first operand that is False or the last operand that is True, and Python's or has always returned either the first operand that is True or the last operand that is False. The only "problem" here is that should someone explictly pass in an argument for students that is False equivalent (empty string, empty list, 0, empty dict, False, etc), you'll still get the fallback operation instead of either no result without error (empty string, empty list, empty dict) or an obscure error further down your code (0, 0.0, False). In Python None is considered False ... so are a lot of things, use it to your advantage, it's not a design flaw. If you really want to be pedantic, then use the ternary syntax, but be nicer to yourself and do Garden.DEFAULT_CHILREN_NAMES if students is None else students ... not None can hurt people.

Me

Ok. This problem you are talking about seems like a very big one to me. If you are used to test for None using shortcuts like this you have a pretty big risk of using it on a boolean variable at some point. I really feel like this is something that we should never do just because it can become an automatism. But maybe the bad habit is just to have some "boolean variable" set to None in the first place?

Mentor

I think you're overestimating the problem of a default of None and a truth test. First because it's very -- even exceedlingy -- rare that you'll have a well-reasoned bit of code that has a parameter that logically should accept any potential false-y but not None argument passed in and treat it substantially differently than None, and second because in professional practice over millions of lines of code I've seen it bite somebody exactly once. And I couldn't and wouldn't describe what broke as well-reasoned.

Me

I'll try to trust you on this one, but I can't promise my paranoia won't catch up ...

This mentor was really thorough and we talk about 3-4 things in such details. To be honest not all mentors will take that much time for you, but a typical exchange will probably teach you at least one thing about the langage.