Should our templates better exemplify best practices?

Programming Python
1

I’ve had the good fortune to have @IsaacG mentor a few of my exercises, and call-out to him, he’s always helpful in both minor and major ways, offering small and large tips for code improvement.

More than once, however, he’s (rightly) pointed out small deficiencies in the code that stem from working with the exercise template and editing it as minimally as possible to complete the exercise.

Two examples from a recent exercise, Simple Linked List’s template:

class LinkedList:
    def __init__(self, values=[]):
        pass

I kept the default value, and my mentor pointed out, “Never use mutable default values!” The link expressed the rationale behind the practice and provided a good alternative.

Is this one worth editing the template for? Doing so would complicate the body of the method, and perhaps require explanation in the docs. My edit based on his feedback and the linked article was this:

def __init__(self, values=None):
    if values == None:
        values = []
    ...

Another example from the same template. His feedback (referring to EmptyListException being defined at the end of the file:

I believe most people prefer defining classes prior to where they are used

So my edit was to place it as the first class definition in the file.

One point of view is that these are minor, perhaps even esoteric, practices (more so with the class definition order, I think). But I also see a case for giving students the best template based on known best practices for the given language. Especially since a major purpose of Exercism is not just to teach programming languages but to teach best practices within each programming language. And having the templates themselves violate some of these best practices seems counter to that goal, especially when stellar mentors correctly point out the violations.

This also might be a higher issue for Python. How many other languages express the language philosophy as import this does? So I’m not sure how specific this issue is to Python (which I’m currently spending my time on) as opposed to other languages.

So my question becomes, as template violations of best practices are revealed to me, should I attempt to edit the template files in the repository to better conform to the best practices?

2 Likes
2

A focus is fluency, not necessarily best practices. I know that I do not mentor "best practices’ because that can come later. Also, depending on if the exercise is learning or practice, best practice might not actually ever come up, current or past best practices.

I think that as you come up against them, consider if having them delivered brings the conversation, or if that (in learning mode) even hides or discourages any conversation about this.

For me, I would rather have a learning of “why decisions are made to do x when y could have been done” rather than always provide, even a template, where we state “This is a “given”, do not modify.” I find that this case, where code is given, most often we are not expecting it to remain as it is. Isaac is familiar with this in other languages, were we sometimes give a “broken” template file, for intents and purposes, the student’s job is to solve the exercise, change what they feel they should or want to change.

3

I think in this particular case, the template code isn’t “broken” in a way that the tests would highlight. As a result, I think many students think “this is set up correctly; it works and I shouldn’t touch it – and maybe I should emulate this code in other projects.” When I suggest improvements, students often point out that this is template code, implying it’s not meant to be changed.

1 Like
4

I can attest to that “I thought…” but I let them know (Ruby track has one exercise that says “Do not change the above code”) that when we do not want a change we state so, otherwise it is their exercise, it is up to them. And “I think” is definitely a wonderful thing, that when that comes to mind, it is a moment to consider “Did I really?”

In a lot of ways, Exercism can let you “it works™” all day long. The thing that brings the value is thinking about the code and doing it other ways, until you get a feel for what you think is right. Then mentoring, and seeing what you might not otherwise see.

5

I can appreciate this with code students wrote or with refactoring exercises. We often tell students not to touch the structure/signature of the functions given to them in the skeleton. Changing those tends to make their life hard. They accept that “this is correct and should not be changed”. I don’t think it’s realistic to expect them to try changing the default value of an argument but not touch anything else about the function signature. “Don’t touch the function signature! Just fill in the rest … except that one bit there ;) That can be played w-- no, that other part there!”

1 Like
6

Hi @chivalry :wave:

Thanks for bringing this up. Truth be told, this exercise stub file hasn’t been looked at or changed in over 4 years - it was ported over from Exercism V2 without any revisions. We then went in and edited directions, hints, and the example solution - but really didn’t look closely at the stub.

I’ve updated and merged the stub in PR 3834.

I can only guess at what the original rational was (I did not write the original stub file). I suspect that because we push the student to not use a list as an internal storage structure, having a list as default argument to __init__() wasn’t thought directly harmful.

Or maybe it was a not-so-nice setup. :woman_shrugging: None (or even no default) is likely a better choice here for stubbing out. The example solution even uses None.

Speaking of that tho…

Your code example contains a different “best practices” violation.
PEP 8 on None:

image
image1596×716 71 KB

The logic of this being that is uses object identity, which is very stable (especially for a singleton). == can be redefined for objects by overriding or changing __eq__(), so is not as stable a method for truly identifying None. So something like this is recommended:


def __init__(self, values=None):
    values = values if values is not None else []
    ...

:wink:

Edited to add

… the larger takeaway being that it’s best to take all “best practices” with a grain of salt, and not get too hung up on them (or too judge-y or preachy).

And that’s the current “philosophy” of the Python track: We don’t want to set up students with something manifestly bad – so yes, we want to correct errors or really misleading stuff (keep the discussion threads coming!!).

– but we also don’t want to foster rigidity, hyper-focus on unimportant details, big debates on style, or prescriptions when it comes to practice exercises. And we really don’t want to imply that there is only one “right” way to code them up. They’re (mostly) purposely designed to have lots of different solutions.

We keep stubs as minimal as possible for practice exercises, and have considered removing them altogether (although that creates troubles with function and method naming). We don’t typehint them or provide docstrings, and we rarely have any notes or comments in them. This LinkedList stub is actually quite verbose.

2 Likes
7

Absolutely about communicating exactly what they are encouraged to change being somewhat of a challenge. In Ruby, we say “Leave the lines above alone to line 14” or something similar for the one exercise I am thinking about, but not directly quoting.

When I mentor, at some point, I ask them to use their exact code from the prior exercise that it builds on.

8

@BethanyG, thanks for the tip. I did know that, forgot, needed the reminder. :slightly_smiling_face:

1 Like
9

Or this as an … interesting alternative :wink: The or operator can do interesting stuff.

def __init__(self, values=None):
    for value in values or []:
        ...
10

…which is why we’ve considered not even stubbing the practice exercises. But I think that would create more confusion, since everyone would start with a stacktrace because the names couldn’t be imported.

So for now we walk the tightrope of some, but not too much code… on a case-by-case basis. :smile:

2 Likes
11

For “practice” the learning to read from the stacktrace is great practice, in terms of becoming fluent. For learning, not so much.

However, if that is how you learned, that is a great incentive to become one of those that work to provide informative, even directive exceptions, and errors.

12

I wouldn’t suggest doing away with the stubs completely. Obviously they’re almost required for the testing, and not a few times I would have designed different were I to start from scratch. For interfaces into classes, a potential alternative (not suggesting this, I generally like the stubs) would be to list names and purposes of methods in docstrings (an example of such in example 6 here).

I would suggest that stubs use general best practices, even if mentors are generally discouraged from pointing out such violations (@IsaacG, please do continue to point out every minor improvement I might make). My assumption, and therefore I’m sure others’, is that stubs are “good code,” so far as they are written, and therefore to be emulated in production code when appropriate.

1 Like
13

I agree to not doing away with stubs completely (in some tracks, and since this is in “Exercism” general category), or in some tracks doing away in later exercises, and even sometimes in both modes, where available and applicable.

Docstrings in Python or comments in other languages, delivered, can be very powerful stuff when used well, I will definitely agree with that. Many students do not read what we provide, some do, some do not on purpose, depending on their goals and familiarity. But comments in source are less likely to be at least not seen.

Mentors are encouraged to mentor how they want to and is a reason I suggest students re-request mentorship or at least not only stick to one mentor. Some of us will focus on certain aspects and ignore other aspects that are just as valid.

14

@chivalry Thanks for helping us make Exercism better.

I think this is really a discussion on (legacy?) python stubs. So I’ll move it to the Python category.

@BethanyG has fixed that specific stub (thank you :blue_heart: ).
@chivalry Are there other specific cases where the stubs don’t use best practices? I always find tangibles lead to better discussions than abstract.

1 Like
15

I looked through the last couple of dozen exercises I’ve done, and the only other instance I could find was the feedback from @IsaacG regarding my Circular Buffer iteration:

Lines 3, 12: PEP-257 says docstrings which are multiple lines should start with a one line summary.

My response to this:

FYI, the docstrings for the exceptions were built into the project template (python/exercises/practice/circular-buffer/circular_buffer.py at main · exercism/python · GitHub).

The violation is that there’s no blank line between the one-line summary and the rest of the docstring.

1 Like
16

Hey @chivalry - thanks for checking and reporting this.

I just took a look at the stub for Circular Buffer on main.

The error docstrings look like they conform to me. What would be the “fix” for them?

1 Like
17

I think the violation is the lack of a blank line after the single-line description.

"""Exception raised when CircularBuffer is full.
message: explanation of the error.
"""

should be

"""Exception raised when CircularBuffer is full.

message: explanation of the error.
"""

I’ll leave it to others to decide if it’s worth correcting.

1 Like
18

I’m not seeing that in the file.:

image
image1980×844 68.5 KB

19

Well, then, never mind. :slightly_smiling_face: I guess the one that originally prompted this post was the only one I’m aware of. Sorry for the confusion.