How to improve this RemoveAt

Question

0.00/5 (No votes)

See more:

Hello everyone,

I need some help to improve this bit of code to remove a node from a linked list at a specific location.

/// <summary>
/// Removes a node from a specific location
/// </summary>
/// <param name="nodePosition"></param>
/// <returns></returns>
public bool RemoveAt(int nodePosition)
{
    //The choosen node is out of bounds
    if (nodePosition > listSize-1 || nodePosition < 0)
    {
        return false;
    }

    //first node was choosen
    if (nodePosition == 0)
    {
        if (listSize == 1)
        {
            first = null;
            listSize = 0;
            return true;
        }
        else
        {
            first = first.Next;
        }
    }

    Node node = first;

    //runs through the list
    for (int i = 0; i < nodePosition; i++)
    {
        //if it is at the beginning or at the end
        if (i == listSize - 1)
        {
            node.Next = null;
            break;
        }

        if (i == nodePosition - 1)
        {
            node.Next = node.Next.Next;
            break;

        }
        else
        {
            node = node.Next;
        }

    }

    listSize--;
    return true;
}

Posted 26-Sep-13 1:00am

Paulo Augusto Kunzel

Add a Solution

Comments

Maciej Los 26-Sep-13 7:10am

What kind of issue do you have? Where are you stuck?

Paulo Augusto Kunzel 26-Sep-13 7:13am

Hello Maciej,
It's simply a matter of performance. I don't think this is the best way to implement it, but so far I couldn't do it better..

1 solution

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Rob Philpott · Accepted Answer · 2013-09-26T02:12:00

That's a very odd way to do it. I wouldn't concern yourself with list size at all.

I'd recommend a two stage approach, first identifying the node you want to remove, the removing it. A good way to keep things consistent is to have an empty dummy node at the beginning of the list, purely to provide the link to the next item (the first item or null) if there isn't one. This way you can write a generalised method which doesn't have to behave differently for the first item.

Node current = dummy; // empty node at front

while (current.Next != null && index--)
{
    current = current.Next;
}

this will stop at the item before the one you want to remove, so to get it you just need current.Next which will be the item or null if you've gone outside the structure.

Then (if current.Next != null):

current.Next = current.Next.Next; // item removed!

It's best when building alogithms to steer clear of special blocks for edge cases (ie. first item, last item etc.)