0

I'm trying to make a StringBuilder like class using linked lists, and I think I'm messing up somewhere in my constructor. Can anyone find an issue here? I believe the issue is with how I'm moving to the next node.

Node class:

private class CNode
{
    private char data;
    private CNode next;

    private CNode(char c)
    {
        data = c;
        next = null;
    }

    private CNode(char c, CNode nextNode)
    {
        data = c;
        next = nextNode;
    }
}

Constructor:

private CNode firstNode;
private int length;

public MyString(String s)
{
    if(s == null)
    {
        this.length = 0;
        this.firstNode = null;
    }
    else if(s.length() == 1)
    {
        this.length = 1;
        this.firstNode.data = s.charAt(0);
        this.firstNode.next = null;
    }
    else
    {
        this.length = s.length();
        CNode node = null;
        CNode nextNode = null;

        this.firstNode = new CNode(s.charAt(0), node);

        for(int i = 1; i < s.length(); i++)
        {
            node = new CNode(s.charAt(i), nextNode);
            node = node.next;
        }
    }
}

2 Answers 2

2

One problem I see is at

this.firstNode = new CNode(s.charAt(0), node);

When that line executes, node is null, so your firstNode ends up linked to nothing. Further, in the for loop where you're trying to build the links, you never assign nextNode, but you try to use it to link one node to the next. Therefore, all the nodes end up linking to null, the initial value of nextNode.

Another issue:

this.firstNode.data = s.charAt(0);
this.firstNode.next = null;

That should be creating a new CNode instead because this.firstNode is still null when that code executes, which will cause a NullPointerException.

Sign up to request clarification or add additional context in comments.

8 Comments

Isn't that the intent, though? It seems like the linked list of characters is stored in reverse order, so the first character shouldn't point at anything. (Unless that's not the intent, in which case that's definitely a problem!)
but I continue to set node in the following for loop? doesn't that fix that problem?
@templatetypedef reverse order? definitely not the intent. how do you figure?
@templatetypedef: That's not how a linked list works, and it's not how he's building the links in the loop (or at least trying to).
@Nealon: See my updated answer. I've pointed out a couple more problems that I notice, including in some problems with your for loop.
|
1

There may be other issues here, but consider this block of code:

else if(s.length() == 1)
{
    this.length = 1;
    this.firstNode.data = s.charAt(0);
    this.firstNode.next = null;
}

Notice that you've never allocated this.firstNode, which means that you're going to get a NullPointerException when executing the second line of code. Try allocating a node before writing to it.

Hope this helps!

3 Comments

replace the second two lines with this.firstNode = new CNode(s.charAt(0), null);?
@Nealon- That should definitely help!
excellent. Still not fixed, but that solves other possible problems. Thank you

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.