patterncsharpModerate
Simple Linked List
Viewed 0 times
listsimplelinked
Problem
I've implemented a simple linked list in C# using generic
As far as my nodes having a previous and next value, that is because I did intend to have quicker searching iterating backwards through, and some other stuff that is not yet implemented. While right now do use both properties of the node, I know I could avoid it. But I have kept it in and use it because I do plan to implement more stuff later.
This is supposed to emulate the List class. I've called mine Jist to not confuse anyone. I also have considered taking away the
```
class JNode
{
public T Value { get; set; }
public JNode Next { get; set; }
public JNode Previous { get; set; }
public JNode(T value) { Value = value; }
}
public class Jist
{
#region Private Members
private JNode _first;
private JNode _last;
#endregion
#region Public Properties
public UInt64 Count { get; private set; }
public T this[UInt64 index]
{
get { return NodeAtIndex(index).Value; }
set { NodeAtIndex(index).Value = value; }
}
#endregion
#region Constructor
public Jist()
{
Count = 0;
_first = new JNode(default(T));
_last = new JNode(default(T));
LinkNodes(_first, _last);
}
#endregion
#region Public Methods
public void Add(T value)
T for values. I did not inherit from any of .Net's fun and helpful classes like IEnumerable, or IList :D. This is just after an hours worth of work. I've tested everything and I believe it all works well enough, but before I go on to add more methods, or make the actual data-structure more complex I'm looking for some feedback on what I have. Critiques on everything are welcome, perhaps there is some glaring issue that I have not addressed.As far as my nodes having a previous and next value, that is because I did intend to have quicker searching iterating backwards through, and some other stuff that is not yet implemented. While right now do use both properties of the node, I know I could avoid it. But I have kept it in and use it because I do plan to implement more stuff later.
This is supposed to emulate the List class. I've called mine Jist to not confuse anyone. I also have considered taking away the
JNode constructor, because all the properties are public anyway, but I've decided against that, because I don't want any accidentally null values... while default values are fine, I don't want any in there by accident.```
class JNode
{
public T Value { get; set; }
public JNode Next { get; set; }
public JNode Previous { get; set; }
public JNode(T value) { Value = value; }
}
public class Jist
{
#region Private Members
private JNode _first;
private JNode _last;
#endregion
#region Public Properties
public UInt64 Count { get; private set; }
public T this[UInt64 index]
{
get { return NodeAtIndex(index).Value; }
set { NodeAtIndex(index).Value = value; }
}
#endregion
#region Constructor
public Jist()
{
Count = 0;
_first = new JNode(default(T));
_last = new JNode(default(T));
LinkNodes(_first, _last);
}
#endregion
#region Public Methods
public void Add(T value)
Solution
Overall, this isn't bad. I do see a few things that bug me though.
-
Code blocks, regardless how many lines they are should be surrounded by
-
The else statement in the following code is redundant because of the
-
Add UL to numbers to avoid having to use the type declaration. This will allow you to use var instead.
-
The variable
- Regions - I find they clutter up the code and don't add much value. I would not use them.
-
Code blocks, regardless how many lines they are should be surrounded by
{}. This makes it much easier for your eyes to line code up. There are a few cases where I think this can be ignored (see point 3)if (node != null)
{
LinkNodes(node.Previous, node.Next);
}
for (UInt64 i = 0; i < index; i++)
{
node = node.Next;
}-
The else statement in the following code is redundant because of the
return. This is one of those cases I'm on the fence about having {} around code blocks.for (UInt64 i = 0; i .Default.Equals(node.Value, value)) return node;
node = node.Next;
}-
Add UL to numbers to avoid having to use the type declaration. This will allow you to use var instead.
for (var i = 0UL; i .Default.Equals(node.Value, value)) return node;
node = node.Next;
}-
The variable
nn is not named very well. Try something like newNode to make the intent more visible.Code Snippets
if (node != null)
{
LinkNodes(node.Previous, node.Next);
}
for (UInt64 i = 0; i < index; i++)
{
node = node.Next;
}for (UInt64 i = 0; i < Count; i++)
{
if (EqualityComparer<T>.Default.Equals(node.Value, value)) return node;
node = node.Next;
}for (var i = 0UL; i < Count; i++)
{
if (EqualityComparer<T>.Default.Equals(node.Value, value)) return node;
node = node.Next;
}Context
StackExchange Code Review Q#44693, answer score: 10
Revisions (0)
No revisions yet.