Discouraged code practice found in Elyses Enchantments

(tl;dr It is risky to modify reference parameters like the exercise requires.)

Hello! I arrived here by way of an excellent (though advanced) book called 99 Bottles of OOP, and I’m trying out the JS track here. The sixth suggested exercise in the syllabus is “Elyses Enchantments”, which introduces arrays and some of its common methods.

I tried to be fancy and return the result in one line, which required that I ignore the normal methods of push() and pop(), so I ended up returning an updated copy of the array and leaving the original alone. For the tests that checked the returned array it worked fine, but for the multi-action tests, modifying the original was expected and so the tests failed. (I have copied one of the relevant tests below.)

const stack = [1];
insertItemAtTop(stack, 5);
insertItemAtTop(stack, 9);
const expected = [1, 5, 9];
expect(stack).toStrictEqual(expected);

Now I remember a time when modifying parameters passed by reference like this was the normal way to have the called function provide a result, but this was long before the establishment of OOP and the dominance of languages that expect it. Nowadays, it can cause all manner of frustration to have a called function modify the data that is sent in since no one expects it and the errors, when they appear, are usually subtle and pop up far from the original function call.

I recognize that this exercise is intended to introduce early concepts and common array functions to fresh coders, but solving this set off a lot of programmer alarm bells for me, and it would be wise to instill standard coding practices from the beginning. With that in mind, I have some suggestions on how to address this issue:

  1. Pre-insert code in the function that copies the array. This does not guarantee that the original array will not be modified, but it does encourage it.

  2. Have the function return nothing. The standard practice in procedural languages that pass back results through reference parameters is to have the functions return void or at least not the object itself. (I can’t think of a single functional language that returns a modified reference parameter.)

  3. Update the exercise description. Looking back, I see the phrase “Return the adjusted stack” in each function and recognize the intention of having them return the original array modified, and yet I can see that phrase also describing the array methods map and filter, which both return copies and not modified originals. Perhaps it can be explicit about modifying the argument?

  4. Change the tests to allow for array copying. If nothing else, please update the tests so that updating and returning a copy instead of the original will pass. Assigning the results of the function call to stack on each line should do it.

It is clear to me that this one example will not dramatically affect how people program, but I wanted to at least mention this issue since, as one person I found online talking about this issue commented, “Modern JavaScript is taking a lot of notes from the functional programming world these days… If I saw you do that in a tech interview… you would definitely not get the position.”

Thank you!

3 Likes

This seems very much Javascript oriented. Other languages, may not have such a strong stance on this. And even other languages may not be able to be solved, and may not be testing, in this same way.

Does this belong under the JavaScript category?

Also… Welcome to the Exercism Forums!

1 Like

Thank you! I’m glad to be here, and I’m enjoying myself so far. I didn’t realize there was a JavaScript category; I will check that out presently.

1 Like

I think that moved the topic over to the Programming > JavaScript category. Thank you again!

Thanks for posting!

There’s a old related issue here FYI: Elyses enchantments: Tests requires a solution with mutations · Issue #1770 · exercism/javascript · GitHub

Bearing in mind @sleeplessbyte’s comment there that we want to ensure array mutation, which of the 1/2/3/4 options above do you think is still relevant.

  1. I think this is a reasonable idea as it then suggests the person still has to mutate. However, we can no longer guarantee they’ve done that
  2. Seems the safest bet with this design goal
  3. Making instructions clearer always seems good regardless of any other changes
  4. Although this is your key point, I imagine will probably meet the most resistance as it’s least likely to guarantee that mutation happens.

We could do the mutation guarantee through the analyzer. If you were interested in working on that, we’d happily take an extra pair of hands on the JavaScript track atm as our main maintainers there are extremely busy with life stuff so things aren’t moving super-fast :slight_smile:

Hi Jeremy,

Wow, I didn’t think I would get your attention! :grin: I’m guessing the site is still at the point where the CEO also sometimes gets to wear the community manager hat? :wink:

Back to the point, it was not at all clear that forcing people to code for mutations was the goal. Even after re-reading the initial lesson on Arrays, the goal I saw was to learn about the basic Array methods, which sometimes requires mutation but not always, and still (I feel) is more safely done on a copy than on the original unless that is very explicitly stated in the function interface (something like, “modify the array argument so that the function caller gets the result through that array”).

To the point about the industry moving toward minimizing using arguments as return values and yet still needing to teach mutability, I think that difference can become a shared purpose of the first array exercise (“we will both learn these methods and show you how to pass results through arguments”) and then shift away from that in later exercises (“once functions start returning arrays, you are not allowed to pass the original array back or even modify it since it’s too easy to change someone else’s array by mistake”). To me, this makes the lesson much more explicit and places the information in a modern context of how people usually use these techniques.

As for the numbered points,

  1. Pre-insert the array-copying code a few times once the goal is no longer to teach mutability. (Maybe use different styles, i.e, [...arr] or Array.from(arr) or arr.slice(), in different exercises to demonstrate that there are different ways of doing it?)

  2. Very important to help distinguish passing the result through an argument vs through the return value.

  3. I very rarely find instructions that over-clarifies the goal and purpose of an exercise (although I have seen many instructions that were badly-written, including ones that were too verbose or ).

  4. I definitely agree that when mutation is explicitly the goal or explicitly not, the tests should reflect and enforce it. (Including Array exercise, task #2 called “Exchange a card in the stack”, which asks for mutation but does not enforce it.)

On the last point you mentioned, I always love to help, especially when it is clear that my efforts are being multiplied to create many blessings across lots of people. What did you have in mind about me helping to maintain the JS stack? Perhaps a phone discussion would be more suited for this kind of request?

1 Like

Hi @DavidOfEarth,

I’d first like to address the actual solutions provided. Jeremy is right that we will not update the test or accept an immutable variant for this specific task as, as this task is how it currently is to force people to use an array mutation, without having to understand the difference between the two. It also is a great conversation starter, just like the one you started :smile:. Whilst updating the description to make something clearer are changes we always accept, I think the most straightforward solution is to implement change two with one extra addition:

  1. Expect void by changing the JSDoc comment
  2. Add an extra test above the current tests for this task (within the same describe block) that checks that nothing (undefined) is returned, and if not, fails with “expected nothing to be returned”.

We’re very happy to take a PR if you’re willing, or if you’re not, please let us know so we can have this change applied.

That said, I find some of the [somewhat hidden] language in your posts misplaced :frowning_face: . To insist that there somehow has been a paradigm shift in programming and that functional languages don’t have mutation, or that OOP somehow changed the fundamental way people deal with reference variables, calling the code required to pass the exercise as “non-standard” or “discouraged” isn’t very helpful. I don’t have a problem with you having a problem with an exercise or an exercise task (please keep disagreeing so we can improve things), but please try to refrain from stating opinion as fact, or using anecdotal evidence in place of research.

Understand that complete beginners with much less experience than you or I will read these posts and think if I learn JavaScript on Exercism I won’t be hired. This is likely not your intention, but even though the message reads as constructive, these opinions and experiences stated as if they are fact are something that I personally strongly discourage.

Of course, we are a teaching and mentoring platform, so I would gladly provide ample example that hopefully helps you understand my point of view, and add to your own knowledge.

  1. LISP is one of the most well-known and earliest high-level functional programming languages. Despite it being functional, it absolute does not enforce immutability. Those who come from SCHEME will happily setf all over the place mutating everything and that will not be considered bad coding practice. In fact, writing performing code in Clojure (which enforces immutability at the language level) is incredibly obtuse compared to something like Common Lisp.
    A language being functional doesn’t mean it’s immutable, and in the same vain, just because JavaScript allows you to apply the functional paradigm, doesn’t mean you must do so in an immutable way.

  2. You indicate that you cannot think of a single functional language that returns a modified reference parameter. Languages that have multiple paradigms, such as JavaScript and Ruby do this all the time.
    In fact, there is a particular function on the Array prototype that does exactly this: Array#sort will sort the array and then return the sorted array reference, but in fact, it sorts the input argument.
    Does this feature cause a lot of bugs? Yes.
    Is that because it’s a really bad idea? No.
    It’s because people don’t know this is how it works, and don’t know why it was implemented this way. The official answer is that it was implemented as Java’s static version and how Perl does it.
    You may think: oh this is the odd one out, but it’s not. Array#reverse and Array#fill are also in-place operations.
    Now as you may know many languages have badly designed or non-designed parts (PHP I am looking at you), so just because the stdlib has it doesn’t make it good, but the fact that these operations are in place make sort, reverse, and fill extremely efficient in JavaScript.

  3. Speaking of efficiency, strict immutability in JavaScript is just not efficient. Almost all efficient implementations (think of things like immer) use mutable objects for all operations until you “commit” the change in order to stay performant.

[1/2]

1 Like
  1. Performance and mutability/immutability are neither topics (concepts) for this exercise, which is why it’s not mentioned anywhere explicitly, but the thought that somehow immutability is “better” (or “newer”) is pretty narrow. That person you found online talking about this really should revisit their opinions because not hiring someone for using a mutable variant (if no constraints are given) is incredibly weird and honestly that by itself is a huge red flag to work for a company that person owns, works for, or works at.

  2. The industry is not moving towards minimizing using arguments as return values. I don’t understand where you’re getting this information from, but I’d like to know, so I can tell them they’re wrong. Please stop making statements like these without backing that up with (preferably peer-reviewed) data. This is also not somehow a modern take on JavaScript. A lot of internal functions, such as Object.keys() were designed from the moment the spec existed to be immutable. This isn’t some new thing people are doing, and I don’t like the framing that “mutating” is somehow “old style” and “not mutating” is somehow modern, because as far as I know there is no data to back that up.

  3. The argument that it’s less safe to mutate would only hold if every other method in this exercise also didn’t mutate, but every single one, except for checkSizeOfStack and getItem mutates the stack. Why would we single out this function?

On a more positive note, the fact that we can rewrite the entire solution to be completely immutable is actually a really nice idea for a practice exercise regarding immutability.

That all said, to reiterate:

  • Happy to make one or several changes to make this task less confusing or frustrating
  • Happy to take a PR, in fact, we’d love that if you feel up like it
  • Happy to discuss this topic further, as long as that statements about the industry or language progression aren’t made as if they are facts unless they are backed up by reliable data.

Thank you for your time thus far! :rocket:

1 Like

Hi @SleeplessByte,

Thank you for your considered response. I basically agree with you on all points, including that it is not helpful for me to cast judgment on the exercise or on the coding practices behind it, that my comment about what is “hireable” could be taken way out of its very narrow context, and that mutability is a critical part of any and all programming languages.

I also want to thank you for contributing so much of your time and effort to helping make this site better, and by extension, helping more fresh learners discover the joy of programming and thinking critically in general.

I can be very explicit that although I have a lot of experience and I draw from a relatively wide base of sources for my conclusions, objectively my knowledge still only covers a small fraction of the whole industry or the coding population, and so I would like to make clear that my thoughts and understandings of how things work can definitely be overturned with thoughtful evidence to the contrary.

As for the topic at hand, I would like to help the syllabus from the beginning guide the learners to balance the motivations involved in design decisions and be exposed to common coding conventions. Specifically, I will provide a PR for where I think the exercise can be more clear and introduce the idea of passing a reference to an object that is shared, modified, and then figuratively passed back, as a foil for a future exercise where immutability is the goal. Before I do, though, I would like to finish the full syllabus so anything I change is in line with the rest of the track.

I hope to work with you going forward, and thanks again for making this site as amazing as it is!

4 Likes

I am looking forward to both things! Object passing and explicitly mutating (and perhaps the interesting thing that JavaScript is not call by reference, but that doesn’t really matter for how we talk about it / understand it) is definitely something we don’t do yet in the entire syllabus, as well as a gigantic list of other exercises that we’re missing.

If you have gone through the exercises and you have specific ideas, please find me on here, through public post or DM, or issues on GitHub, because extending the syllabus is something we want, but can use every bit of help with!

1 Like

Ha - probably sometimes although @jonathanmiddleton is doing a stellar job! But mainly more that I think it’s essential that I’m is as close to the product as I can be, so hearing thoughts, concerns and ideas is critical. Plus, I love programming so this stuff is inherently interesting to me :grin:

@DavidOfEarth @SleeplessByte Thanks for the positive conversation! :blue_heart:

2 Likes

@SleeplessByte @DavidOfEarth @iHiD

Just wanted to say that this conversation brought such joy to my heart! The tone, depth, humility and overall feel was exactly what we want the forum to look like.

Awesome!

1 Like

The fact that it does not force immutability means that mentoring can take more than a single route, as well, and the discussion can happen. This is not as helpful, perhaps, for concept exercises, but for practice exercises, the flexibility is something I consider an awesome feature. It means that we can visit again, for a very different experience.

2 Likes

Absolutely. And since this is a concept exercise we try to force the one route :slight_smile:

I would like to DM you, but I can’t seem to find the interface element for it. Can you point me in the right direction? Thanks!