exercism / python

Exercism exercises in Python.
https://exercism.org/tracks/python
MIT License
1.94k stars 1.29k forks source link

[Little Sister's Vocabulary] exercise uses lists + other issues #2957

Closed mohmad-null closed 2 years ago

mohmad-null commented 2 years ago

https://exercism.org/tracks/python/exercises/little-sisters-vocab

def make_word_groups(vocab_words):
    """

    :param vocab_words: list of vocabulary words with a prefix.
    :return: str of prefix followed by vocabulary words with
             prefix applied, separated by ' :: '.

    This function takes a `vocab_words` list and returns a string
    with the prefix  and the words with prefix applied, separated
     by ' :: '.
    """

There are two problems here:

a) It requires the user to manipulate a list before you've introduced lists.

b) If I saw this in real-world code, it would flag my code-smell detector. We should be teaching people good code practices. The function definition should be: def make_word_groups(prefix, vocab_words) - the prefix should be a separate parameter. Although that would then complicate the solution (as loops haven't been taught yet), I guess get rid of the prefix entirely and just teach join.

github-actions[bot] commented 2 years ago

🤖   🤖

Hi! 👋🏽 👋 Welcome to the Exercism Python Repo!

Thank you for opening an issue! 🐍  🌈 ✨


​          ◦ If you'd also like to make a PR to fix the issue, please have a quick look at the Pull Requests doc.
             We  💙  PRs that follow our Exercism & Track contributing guidelines!


💛  💙  While you are here... If you decide to help out with other open issues, you have our gratitude 🙌 🙌🏽.
Anything tagged with [help wanted] and without [Claimed] is up for grabs.
Comment on the issue and we will reserve it for you. 🌈 ✨

BethanyG commented 2 years ago

Hi @mohmad-null 👋🏽

a) It requires the user to manipulate a list before you've introduced lists.

Please see the reasoning laid out here in closed issue #2588. As long as there are clear examples, it is not forbidden to add in an additional concept. I would also say that having .join() unpack an iterable is not the same as having a student use list.sort() or list.append(). So we are not asking students to manipulate lists - only understand that .join() can process one and that str.split() returns one.

b) If I saw this in real-world code, it would flag my code-smell detector. We should be teaching people good code practices.

I'm not sure how breaking down data in one form to return it in another is a "code smell". Sometimes, separators are well-known and fixed. This one is clearly stated in the task. Dealing with how or why you would have a parameter for a variable separator or a fixed one (or pulling one from a config file or any other permutation) is out of scope for this exercise.

edited to add: I mis-read that you were speaking about the word prefix, but my statement still stands -- the student has been given instruction on how to extract an element from both a str and a list at this stage, since they are both sequence types. So this should not be a huge struggle for them. I'd also argue that weird input and output data is indeed a "real world" thing - so having to pull out a prefix and then transform the rest of the list and return it with a separator in a rather arbitrary feeling str is a "real world" scenario. 😉

mohmad-null commented 2 years ago

I raise these issues because not only did I struggle with them (as a result of the instructions not being entirely clear and the code-smell), but also because a mentee struggled too.

As long as there are clear examples, it is not forbidden to add in an additional concept. It should be. The purpose of these exercises is to teach, and to do so effectively. That means not putting in other concepts that are not pertinent; that will simply add confusion to the exercise. Teach one thing and teach it well.

I'd also argue that weird input and output data is indeed a "real world" thing Indubitably, but so is crime. That doesn't mean we should expose people to it if we can avoid it. And absolutely definitely not during a teaching exercise.

Yes there are countless real-world examples where you have to pull data out of itself and then munge it back together again (practically my day job). I'm not an academic so I can't expound on the theory, but the current "use the first value to edit all the other values but not the first value" feels onerous. The fact that it's not split out before the function definition feels code-smelly given this is a entirely contrived example rather than real data..

BethanyG commented 2 years ago

@mohmad-null I think I am going to stop my participation in this discussion here, as I don't think we are going to come to an agreement. I've asked several other maintainers to take a look, and weigh in on what they think of this exercise, its "code smells", and where it's placed in the exercise tree.

Meanwhile, I'd encourage you to submit a PR with changes you feel improve this exercise, or teach the concept in a better fashion from your perspective.

github-actions[bot] commented 2 years ago

This issue has been automatically marked as abandoned 🏚 because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.