Simple list with LIFO behaviour

[michalporeba]

Discussion on the forum: (link).
The problem: The exercise Simple Linked List (specification) explains simple linked list as something with Last In First Out (LIFO) behaviour. It is at odds with what is currently the reference implementation in C# track, and what is required to pass the tests.

The track's expected behaviour is First In First Out (FIFO). At the same time, the implementation, while technically correct, it is less classic, especially in the context of object oriented language like C#. There is no encapsulation of the list internals which can lead to problems with references should LIFO behaviour be implemented. Imagine the below situation:

var a = new SimpleLinkedList<int>(1,2,3);
var b = a;
/* at this stage both a and b point to node with value 3 */
a = a.Remove();  /* LIFO pop operation */
/* now a points to 2 but b still points to 3 */

That is why the common implementation (examples from other tracks: go, c++, java) uses a container external object encapsulating the linked list logic. As you can see in the provided ode the implementation is actually simpler than doing it without encapsulation.

The solution: What I'm proposing means big changes, but it bring the c# track in line with tracks from languages with similar philosophies while at the same time maintaining the .net / C# feel by maintaining IEnumerable, using Add(T value) and Remove() rather than Push() or Pop(). It introduces or exercises the concept without forcing solutions that have significant problems (hanging references).

This is not finished product. I have done minimal work on the tests, only enough to make them work with the new structure again. If this is the direction we want to go in, I will work on more comprehensive test suite.

Open questions:

• Is this a step we can take considering all the existing solutions would be invalidated (but future user will have better learning experience)
• Should we introduce Count() function in line with the problem spec?
• Is Remove the right method name? In C# typically it is used to remove a specific element from a collection. Perhaps RemoveFirst() would be an option, or perhaps building on top of IEnumerable we could use Next()? (This would not work as iterating over the list would empty it!)

[github-actions]

Hello. Thanks for opening a PR on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed.

That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there.

---

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.

[michalporeba]

When you look at the example, please keep in mind that we should discuss the approach rather than treat it as a code review at this stage.

[ErikSchierboom]

@michalporeba I haven't yet found the time to look at this, sorry!

[michalporeba]

No problem, @ErikSchierboom, take your time.

[ErikSchierboom]

I would prefer to first have the student use Add explicitly.
I wonder if actually we should not even have any constructors, just focus on Add, Remove and the IEnumerable logic. Your thoughts?

[michalporeba]

I would like to build this exercise more with more granular steps, perhaps using Add/Count or Add/Remove first. Definitely Add makes sense to start with.

The constructors are, in my opinion, very idiomatic for .net so it would be good to keep them, but they could be a separate step with some narrative to help understand why.

I'll propose something soon (I hope, if not then next week as I will be mostly offline Thursday to Sunday).

[ErikSchierboom]

The problem with the constructor is that the tests could force the students to write them before they can even get tests running. Unless we do so reflection.

[michalporeba]

I think the tests should follow the TDD style, and with steps they can. The C# track already does it so much better than many other tracks I have tried, and with reflection at our disposal we can easily make it work great leading the student through the exercise step by step.

If you don't mind @ErikSchierboom, I'd like to give it a go, implement something like this so we can talk specific pros and cons on an example.

[ErikSchierboom]

Should we introduce Count() function in line with the problem spec?

I like that!

Is Remove the right method name?

I think it is usually called Unshift, but that's not a great name either. RemoveFirst or RemoveHead could work.

[michalporeba]

I think it is usually called Unshift, but that's not a great name either. RemoveFirst or RemoveHead could work.
Well... it is in a doubly linked list. In a simple list like this I believe it is push and pop. But that is not so relevant in C# I think. Microsoft was rather consistently ignoring this standards favouring Add and Remove or Enqueue and Dequeue or similar words for similar operations throughout their libraries and tools. The Stack<T> implementation with Push, Peek and Pop is the exception.

The added challenge is that Remove... normally do not return. In that regard dotnet follows command query separation. Both Add and Remove cannot be used to get a value.

I see we have three options to implement the spirit of the exercise and keep dotnet ways of doing things

Option 1 - be stack-like

The list behaves like a stack (LIFO) and so we could adopt the stack methods Push and Pop

Option 2 - follow list naming and add GetFirst

Have three methods - Add, RemoveFirst and GetFirst.

Option 3 - GetAndRemoveFirst

Some .net libraries opt for methods like GetAndRemove breaking the command query separation in a very explicit name. We could have GetAndRemoveFirst method

To be clear, for me option 3 is the least favourite by some margin. But I cannot make up my mind between options 1 and 2.

[ErikSchierboom]

Personally, my preference would be to go with option 1. I think we'll get the cleanest interface that way.

[michalporeba]

The biggest change compared to what is there now, but the cleanest end result. I'll got with that.

[ErikSchierboom]

The biggest change compared to what is there now, but the cleanest end result. I'll got with that.

Great! Let me know when it is ready to be reviewed.

[michalporeba]

@ErikSchierboom I got to the point where I need some help. I think the current code could be a good starting point. Constructors are not needed, the IEnumerable<int> implementation is not yet needed, and all that is needed for the first task to pass is to add

public int Count { get; private set; }

But because there is no stub for the property Count the automated tests on the track will fail. What should I do here?

Also, while I have added [Task(x)] attributes I haven't added instructions. I looked at a few other exercises with steps, and looked for documentation but I don't know what is the best / current / recommended way to do it, especially considering the efforts to standardise descriptions. Can you help?

[ErikSchierboom]

But because there is no stub for the property Count the automated tests on the track will fail. What should I do here?

Add a stub :) I think the default stub could be public int Count => throw new NotImplementedException("Please implement this");

Also, while I have added [Task(x)] attributes I haven't added instructions. I looked at a few other exercises with steps, and looked for documentation but I don't know what is the best / current / recommended way to do it, especially considering the efforts to standardise descriptions. Can you help?

You can remove those. Practice exercises don't yet have tasks associated with them, although we are indeed try to start with that. For now, I'd not have them.

[michalporeba]

The stub is in place, tasks removed and some information added to help guide the implementation.

The previous version of the instructions stipulated that input will be correct, now this has been removed. As such, and for completness of the solution should we expect that Pop() throws InvalidOperation when there is nothing in the list?

[ErikSchierboom]

As such, and for completness of the solution should we expect that Pop() throws InvalidOperation when there is nothing in the list?

Yes please!

[ErikSchierboom]

Nice!

[ErikSchierboom]

This is not something that students need to implement as they get it for free by implement IEnumerable, right?

[michalporeba]

If they do it right. If they don't then the test will fail.

[ErikSchierboom]

I think this is great!

[ErikSchierboom]

Are you okay with me merging this?

[michalporeba]

Yes, please!

I'd like to submit more PRs for other things, but it looks like there are many open and it would be good to close them and have perhaps smaller changes to avoid work with rebasing and so on. Sort of limiting the work in progress (WIP).

[ErikSchierboom]

That sounds great.