Hello, I am a beginner in Kotlin and in general st...

# codereview

[JP]

Hello, I am a beginner in Kotlin and in general statically typed language. I wanted to practice the OOP concepts using Kotlin, so I wrote some code for implementing singly linked list (one mutable and one immutable) as Kotlin’s Collection, referencing some codes from Kotlin’s source code. I’m yet not so confident with how I structure the code using interfaces and classes, so I’d like to get some comments. Thanks a lot in advance.
An exemplary question that arose was:
should I have used abstract class in between? (I am not yet sure what the differences between interfaces and abstract classes are, and when to use which.)

SinglyLinkedLists_kt.kt

[Kroppeb]

Welcome to Kotlin. TBH there are a quite some issues here, so I'll go over them.

`ListNode`:
* use something like value instead of `\`val\`` so you don't need
* this is a mutable object which you use in SinglyLinkedList meaning that one isn't 'immutable'. One way to fix this would be to have a ListNode and a MutableListNode
* You want this to be either an interface or just don't expose this to the user at all.

On the topic of making a "immutable" and a "mutable" variant, the more correct way to look at it is "mutable vs read only" where read only can still be mutated by other objects (like owners and stuff), although you can often cast them back to the mutable state, this is considered bad practice. To achieve this you correctly made the mutable interface extend the read only one.

`ISinglyLinkedList`:
* In kotlin we don't use the I- prefix unlike in C#. I would like it but it would become really inconsistent when working with java interfaces.
* head and tail are exposed as vars and are therefore mutable
* I don't really see why a tail is provided to the user
* You are providing head and tail as a ListNode. I don't see why a user would ever need to use the node instead of the value itself.

`SinglyLinkedList`:
* the way you implemented isEmpty makes it look like you expect size to be 0 while there is data. This seems wrong to me
* There is no reason for this class to exist

`SinglyLinkedListIteratorImpl`:
I don't think you should have an interface for this one. Implementationwise, I have no idea what you are trying to achieve, and why there is both a currNode and a prevNode

[Kroppeb]

This is how I would have done it

Untitled

[JP]

Thanks a lot for the opinion and taking the time to look at the code. It’s a great help for me. The thing with the ListNode is that I was practicing originally problem solving at some website with Kotlin, and there’s the predefined ListNode class with member name like ``val`` . And I was trying to build from that ListNode class.

[Kroppeb]

AbstractMutableCollection Adds most implementations using the iterator, which is in a lot of cases way slower as it's almost always at least`O(n)`. So use AbstractMutableCollection in case you don't care about performance and need something quick that doesn't take too much effort to make, or use it and override the methods only when you need them a lot.

[Kroppeb]

AbstractMutableCollection is just a kind of helper where the method implementations are often ok but not great.