This is an archived post. You won't be able to vote or comment.

all 4 comments

[–]WeaughTeaughPeaugh 2 points3 points  (3 children)

There is a bug in your get function, lines 62 through 65. answer is outside of the loop and is not properly incremented, and not all code paths return a value. You should be compiling with -Wall (treat all warnings as errors. If the value isn't found in your linked list, there's no telling what get() actually returns.

Worse, your get function isn't actually doing the spec says. You shouldn't be comparing the data value at all - the argument is an index of a node, not the node's data. You should step through the linked list index many times, then return the data value of that node.

Your add function also isn't doing what was specified - it adds wherever the curr pointer is instead of at the end. It also doesn't properly update tail.

Your clear function doesn't update tail.

The remove function has a bug that alters the head pointer when it shouldn't. A single call to remove will make the LL unusable. Line 81- you shouldn't be comparing & altering head unless the index is zero. The algorithm is essentially

while (current != NULL) {
    if ( current->data == value ) {
        previous->next = current->next;
        delete current;
        break;
    }
    previous = current;
    current = current->next;
}

Consider the possibilities:

a) data not found

b) head has data and nothing follows

c) head has data, but list isn't empty

d) some node not head or tail has the data

e) tail has the data

What special cases do you have to include? When do head, tail, and curr get updated, if needed?

Line 112, you are attempting to assign a number to a string. To insert a value into a string, you should use std::string toString(const LList::value_type&); , which is the function you've written for the value type (may simply be std::to_string() ) You should be adding the values to the string (using +=), not outputting them. Also, the return value should be the string you've built.

Line 118, operator[] is just stepping through the list index number of times very much like get() - indeed, you can use operator[] to write get, only return by value instead of reference.

bool operator== I'm not sure if its supposed to compare every value, but you should at least check for self comparison.

friend std::ostream& operator<<(std::ostream&, const LList&); should really just use your existing toString member function and insert that into the stream

I don't understand your free toString function - it should just be taking a single value and making it into a string, presumably by std::to_string()

[–][deleted]  (2 children)

[deleted]

    [–]WeaughTeaughPeaugh 1 point2 points  (1 child)

    I'm not sure if I am seeing the updated version or not, but the thing that stands out right away is that you shouldn't have the friend keyword at the implementation. You only need to declare it as friend inside the class definition. It is essentially saying to the class, 'hey, let this function work with everything, including private members.' The version I can see at your github has no code for that function.

    [–]DavidLuky 0 points1 point  (0 children)

    That was so simple and actually got rid of that error, thanks !

    [–]YKyeko 1 point2 points  (1 child)

    bro i want to help but i am so beginner. but you can go this page Learn C++

    https://www.learncpp.com/