Add list-ops exercise (WIP)

[kahgoh]

This adds the list ops exercise for 48in24 and is based on previously done by bobbicodes under #552.

Also contains a fix for the location of the CI example location in the config.json and formatting config.json as applied via configlet.

[github-actions]

Hello. Thanks for opening a PR on Exercism 🙂

We ask that all changes to Exercism are discussed on our Community Forum before being opened on GitHub. To enforce this, we automatically close all PRs that are submitted. That doesn't mean your PR is rejected but that we want the initial discussion about it to happen on our forum where a wide range of key contributors across the Exercism ecosystem can weigh in.

You can use this link to copy this into a new topic on the forum. If we decide the PR is appropriate, we'll reopen it and continue with it, so please don't delete your local branch.

If you're interested in learning more about this auto-responder, please read this blog post.

---

Note: If this PR has been pre-approved, please link back to this PR on the forum thread and a maintainer or staff member will reopen it.

[bobbicodes]

This feels a bit like I'm reviewing my own PR, but assuming the tests pass, and that little note to the implementer is removed, I think it should be good.

[bobbicodes]

I think we can just delete this line because we implemented all operations.

[kahgoh]

@bobbicodes The contents of this file comes from the problem specifications, which is also used by the other tracks. I think we would need to update it in the problem specification to remove it, but that could effect the instructions in the other tracks when they are re-synced.

[BNAndras]

@kahgoh, are you still interested in working on this PR with the new Clojure maintainers?

[kahgoh]

@BNAndras Yes, I still am! Its been a while since I looked at this, but I've just tried merging the latest main to my branch and the build is failing. I'm guessing something has changed since I last looked. I'll have a look into this soon.

[kahgoh]

Ok, I think I've worked out what was wrong and the CI seems to work now. Ready for another review!

[BNAndras]

@exercism/clojure, for your review. :)

[tasxatzial]

@kahgoh I do a proper review soon, in the meantime:

• The file generator.clj should be re-added, as it appears to have been deleted.
• The ;; <- arglist goes here comments in list_ops.clj are unnecessary, as the arguments are already provided.

[tasxatzial]

Initially, I thought the exercise was fine. All the files appeared to be there, the tests were written correctly, and the existing implementation passed. Great. However, I'm now realizing that there are some issues. Let's go over them one by one.

• The instructions specifically mention functions that operate on lists. But what do the tests use? Vectors! Hmm, i can definitely see this being confusing to many.

• The instructions mention:

  Implement a series of basic list operations, without using existing functions.

  What does that mean though? Let's look at list-ops.clj:

  Line 6: (empty? l). It might be better to use length here.

  Line 10: (conj acc elem). Isn't conj another word for append?

  Line 31: (cons elem acc). Same problem as before.

  Obviously, the implementation itself doesn't really matter as long as it passes the tests. However, it does highlight that it's unclear what kind of implementation is expected.

• Alright, let's solve the first problem by switching to lists in the tests. Run the tests, and hmm, now the existing implementation fails? Whoops! The conj function is to blame, as it behaves differently on lists versus vectors.

• The instructions mention:

  The precise number and names of the operations to be implemented will be track dependent to avoid conflicts with existing names.

  Currently, the functions concat, filter, map, and reverse conflict with existing names. This might not seem like a big problem until someone realizes, for example, that in the current implementation, the reverse function needs to be defined before foldr so that the program uses our implementation instead of the built-in reverse. It's very easy to accidentally reuse an existing function when our intention is to use our own. Luckily, this can be easily solved by simply changing the function names.

• Some functions are using list as their argument, which also conflicts with the existing list function. It's not a big problem, but it should be avoided if we're talking about good practices.

• Lastly, the exercise is currently marked as easy (3). Given that there's a lot of code to write, with many moving parts and concepts, and that it's probably a bit tricky to implement properly, I would bump the difficulty up to medium (5).

I'm not going to ask for specific changes right now, as I'd like to think about what can be done to make the exercise a bit clearer and less confusing. Suggestions are very welcomed :)

@kahgoh Do you know if it's possible to avoid conj and cons?

[BNAndras]

My thoughts as a maintainer who implemented this a few times elsewhere...

An instructions.append.md would go a long way in clarifying the track-specific implementation details you're highlighting. It can just be a few sentences saying this exercise is in fact using vectors on the Clojure track, provide some documentation links about vectors, and maybe a code example or too of working with vectors. That should give enough info to get students started.

It'd also be a good idea to avoid shadowing built-ins as you mentioned. That's a practice we probably don't want to encourage in general. It also potentially limits the approaches a student can use if they can't easily access a built-in. Practice exercises typically have more than one possible way to approach them so it'd be good not to restrict the students. Concept exercises can be more restrictive on how to solve them because they're reinforcing a particular concept.

[tasxatzial]

@BNAndras An instructions.append.md is definitely the way to go. However, I'm not sure whether we should stick to using only one type of container (whether vectors or lists doesn’t really matter), or if it’s worth the effort to extend the tests to cover both. Doing so could potentially open the door to more general and interesting solutions that work with any type of collection, which is very much in line with the Clojure way of doing things. Most likely, we'll just stick with what we already have, as I feel it’s not really worth the effort.

My main concern here is whether we should instruct people to avoid using cons, conj, or any similar function, as they largely defeat the purpose of the exercise. I'll take a look at solutions from other tracks—maybe we can get some ideas.

[kahgoh]

Do you know if it's possible to avoid conj and cons?

Looking at the Emacs lisp, Scheme and Racket I don't think there is a way to avoid cons or conj. I'm not sure if I am missing something (I barely have any Clojure experience), but I thought using cons / conj would be fine because the instructions define append to add the contents of a list to the end of another, instead of adding an element to another list.

[BNAndras]

A student using built-ins for this is only cheating themselves at the end of the day. From what I recall, the Crystal track customized their test runner to block common built-ins for particular exercises.

That might be an approach here, but I’d also recommend warning the student in the instructions append to not use built-ins. It’d be frustrating to put together a passing solution just to fall a hidden requirement.

[tasxatzial]

@kahgoh Good points, thank you. It's not worth making any changes then. I'll suggest the final set of changes, at least from my side, tomorrow.

@BNAndras Thanks. A warning in the append file seems appropriate.

[BNAndras]

The strain docs warn folks to try implement the exercise without using the standard library. It might be useful to update the upstream docs for this exercise. In a few mentoring discussions for this exercise, it's been recommended before to not use the standard library. You'll want to start a forum thread to discuss that potential edit before making a problem-specifications PR. :)

[tasxatzial]

@kahgoh

Let's rename the functions and arguments to avoid conflicts. I propose the following:

• concat -> concatenate
• filter -> select-if
• map -> apply-to-each
• reverse -> reverse-order

Also, let's rename the arguments to be more in line with what we see in the official Clojure docs:

• list -> coll
• list1 -> coll1
• list2 -> coll2
• f (only in the filter function) -> pred

Final change is about adding docstrings to all functions so that it's clear what they do. We can use the instructions for this, but we should avoid any reference to lists. For example:

(defn select-if
  "Given a predicate and a collection, it returns the collection of all items for which predicate(item) is true"
  [pred coll]
  ;; your code goes here
)

---

Please change the exercise status to "wip" and bump the difficulty to medium (5).

I'll be adding an append file myself later with some implementation tips and to clarify any issues.

[tasxatzial]

The reason I asked to avoid mentioning lists was that the tests are using vectors, and I didn't want people to get confused. At the end of the day, it doesn't really matter whether we chose vectors or lists. So, we are being purposefully vague here by using the term 'collections' instead of 'lists'.

Another option was to change the tests to use lists, but then you'd have to rewrite the example solution.

Also, the docstring in foldr was after the arguments (the correct place is before).

Sorry, i'm being a bit pedantic here. All my suggestions can be applied directly to save you some time.

[kahgoh]

Sorry @tasxatzial, that was my bad, as I had missed the bit about avoiding reference to lists in the earlier comment.

[kahgoh]

I've just realized the tests are using the term "lists" in the description. For example, the first test for append description is empty lists. Should we change these to collection too? I think it would be less confusing if we did, but then it wouldn't match the problem specification...

[tasxatzial]

If you say that it wouldn't match the description from the problem specs, yes, that's indeed an issue. However, the term 'list' doesn't necessarily imply that we should be using Clojure lists. The language might not even have lists.

With all that said, I'm now realizing that being too vague and instructing in the append file to assume either lists or vectors (which was my plan) might do more harm than good. I expect many people to adjust their solutions based on their assumptions, which means they might solve the exercise differently (*). This, in turn, could confuse those who browse the solutions. That doesn’t sound like a good idea. I'm leaning towards changing the docstrings and test descriptions to use vectors. A note in the append file will tell people to assume vectors. What do you think?

(*) An implementation that uses lists appears to be more complex than one that uses vectors. Adding to the end of a vector can be trivially done with conj, but something similar isn't possible when using lists.

[kahgoh]

I think changing the doc strings to vectors and telling people to assume vectors in the append makes sense. This would be consistent with the tests, which uses vectors. Telling them to assume either lists or vectors might also be confusing if the tests uses just vectors.

[tasxatzial]

You know, in all exercises so far, the tests use vectors for the return type, but they also work fine if someone returns a list. None of the existing exercises have a note about this. People are free to choose whatever they want, but in this particular exercise, a clarification is obviously needed. Also, iterating over a vector is done the same way as iterating over a list—the foldr implementation is an example of this. So, you can assume that the function input is a list, and write a solution that passes the tests. This is why I tried to use the abstract term 'collection,' because Clojure functions operate in terms of collection and sequence abstractions. A note in the append file can address this, but I agree that, as you said, it's still a bit confusing.

In any case, I’m afraid using 'collections' might be aiming too high for two reasons:

• Using vectors in tests does seem to make a difference in terms of what kinds of solutions are expected. It feels like solving two different exercises, even though the solutions will look similar.
• If people assume either vectors or lists, they might devise a solution that accepts vectors and returns lists. Even the reverse-order function in the current example solution does that—it accepts a vector and returns a lazy sequence. But since the exercise talks about functions that accept type X and return type X, a solution that doesn’t maintain that could be interpreted as not meeting the requirements.

Let me think about it for a couple more days. If nothing new comes up, we'll simply rename all references to 'collection' or 'list' to 'vector,' add the append file, and call it a day.

[tasxatzial]

@kahgoh Let's make the final changes. Please rename every reference to collection/list to vector in the docstrings and test descriptions in list_ops.clj and list_ops_test.clj.

No need to rename the function parameters; they should be perfectly fine as they are.

[tasxatzial]

Some of the previous changes have been undone by the most recent commits.

[kahgoh]

Ah sorry, that was rather clumsy of me. Fixes applied.

[tasxatzial]

Don't even think about it. These things are bound to happen.

I've added the append file, please take a look.

Edit: Also changed 'series' to 'vector' for consistency.

[kahgoh]

I just had a look through. It looks good to me.

[tasxatzial]

Alright, this exercise has come a long way. It has likely been one of the trickiest to implement due to the list-to-vector change. Also, if I'm not mistaken, it's the first exercise with an explicit instruction to use vectors. Let me sum up the changes:

• We changed the function names and parameters to avoid conflicts.
• Consequently, we added docstrings to clarify what each function does.
• We clarified that this exercise is about vectors as both input and output, to align with the problem specs.
• We avoided using certain built-in functions and listed them in the append file.
• We changed the difficulty to medium, which can be adjusted in the future based on the community solutions.

A big thank you to everyone who got involved:

@bobbicodes for the initial implementation.

@kahgoh for his patience and willingness to continue Bobbi's work.

@BNAndras for his insightful comments.

[kahgoh]

I just remembered to add @tasxatzial to the exercise contributors (for the writing the instructions.append.md and other suggestions made).

[tasxatzial]

Thank you :)

Regarding the edits to the generators from Bobbi's older PR, I haven't forgotten about them. I'm planning to add them in a separate PR to keep this one cleaner.

1. Maintainer not active 2. The requested change will be undone when docs are resynced.