0

I'm learning c++ and am attempting to maintain a linked list in alphabetical order. If I were to input a list of names say "Mary, bob, sally, larry, david, roger" I would expect them to print out (after traversing the list) "bob, david, larry, mary, roger, sally". Any help would be greatly appreciated. Thank you!

Note: The code is not printing the names in alphabetical order when I attempt to execute it. Apologies, thought I wrote that, but apparently I did not. It compiles fine, but the results produced are not what is expected.

EDIT: I noticed something interesting. I'm receiving an error when I attempt to build the file c:/mingw/bin/../lib/gcc/mingw32/4.9.3/../../../../mingw32/bin/ld.exe: cannot open output file Program18ContactList.exe: Permission denied collect2.exe:error: 1d returned 1 exit status It runs, but when i attempt to rebuild any changes I get that message. I get the idea of what it means, but am unsure how to fix it. I haven't deleted any source files. I am using the newest version of Eclipse Mars.2

Main

//Contact list playing with classes
//Contact lists

#include "ContactList.H"
using namespace std;


int main()
{

    ContactList* cl1 = new ContactList();

    string name;

    while(true)
    {

        cout << "Enter the name of the contact or q to quit." << endl;
        cin >> name;
        if(name == "q")
        {
            break;
        }
        cl1->insert(name);

    }

    cl1->printList();

    return(0);
}

Class

//contact list class

#include "ContactList.H"
using namespace std;

ContactList::ContactList():head(0), size(0)
{}

void ContactList::addToHead(const string& name)
{

    Contact* newOne = new Contact(name);

    if(head == 0)
    {
        head = newOne;
    }
    else
    {
        newOne->next = head;
        head = newOne;
    }

    size++;
}

void ContactList::printList()
{

    Contact* tp = head;

    while(tp != 0)
    {

        cout << *tp << endl;
        tp = tp->next;

    }

}

void ContactList::insert(const string& name)
{

    Contact* newNode = new Contact(name);

    // case 1 - empty list
    if(head == 0)
    {
        //assigns new node to the empty list head node
        head = newNode;

    }
    else
    {
        //current pointer initialized to the head so as to start traversal
        // trail pointer initialized to zero, but will iterate one step behind
        // current to keep former data location on the stack organized
        Contact* curr = head;
        Contact* trail = 0;

        // Traverse list to find insert location
        while(curr != 0)
        {

            if(curr->name >= newNode->name)
            {
                break;
            }
            else
            {

                trail = curr;
                curr = curr->next;

            }
        }

        // case 2 - insert at head (not empty)
        if(curr == head)
        {

            newNode->next = head;
            head = newNode;
        }
        else
        {

        // case 3 - insert after the head (not empty)
        newNode->next = curr;
        trail->next = newNode;
        }

    }

    size++;
}
StormsEdge
  • 854
  • 2
  • 10
  • 35
  • 2
    Have a look at [std::list](http://www.cplusplus.com/reference/list/list/) and [::std::sort](http://www.cplusplus.com/reference/algorithm/sort/). You *might* have to pass a comparator to std::sort, though, using `x.compare(y) < 0`. – Aconcagua Jun 07 '16 at 14:33
  • Forget about the custom comparator, according to [here](http://stackoverflow.com/a/688068/1312382), it is not necessary... – Aconcagua Jun 07 '16 at 14:37
  • What is failing with your current code ? After a very quick look it seems to me that the code should work as expected ... – VB_overflow Jun 07 '16 at 14:43
  • 3
    @Aconcagua: For real code, one would use std::vector and either sort it after insertion, or use std::lower_bound to keep it sorted. This is obviously an exercise for a student - as such using the standard library defeats the point of the exercise. – Martin Bonner supports Monica Jun 07 '16 at 14:43
  • @MartinBonner Depends on your requirements - if you are inserting and removing a lot in the middle, lists can be of advantage. One great thing about lists: iterators do not invalidate if you insert or erase elements at any location (except for erasing the item the iterator points to, of course...). – Aconcagua Jun 07 '16 at 14:48
  • 1
    I can't see what is wrong with your code, but a general rule when dealing with lists is **no special cases**. So start out with `Contact **target = head;` search through the list (if any) to find where to insert the new element, and set `target = &curr->next` whenever you don't break out of the loop, then `newNode->next = curr; *target = newNode`. This will handle 1. Empty list; 2. Insert at head. 3. Insert in middle. – Martin Bonner supports Monica Jun 07 '16 at 14:49
  • @Aconcagua: If you need the iterator guarantees, then yes, absolutely. If you need performance, then vector will usually win because it is *so* much more cache friendly. On big-O list wins, but the constant factor of vector can be a orders of magnitude better. (But as always with performance: test first with realistic datasets to get the actual answer). – Martin Bonner supports Monica Jun 07 '16 at 14:52
  • Step through your code in a debugger, and see where it goes wrong. I would suggest a dataset like: "charlie alice bob eve". This 1. Inserts in an empty list. 2. Inserts at head. 3. Inserts in the middle. 4. Inserts at the end. – Martin Bonner supports Monica Jun 07 '16 at 14:54
  • Please see edits up top. @NathanOliver Don't really want your help if you're going to be rude. I wasn't clear enough which I realized. I've posted plenty on SO and on Code Review. I'm reading through these comments and will try them all. Thanks for everyone's help. – StormsEdge Jun 07 '16 at 16:34
  • Do you need the list to be sorted automatically when adding and removing entries? If so, then you could make a wrapper around a `std::list` or `std::vector` and use a custom comparator for comparing `std::string` objects to sort the list/vector every time a modification operation occurs. – sjrowlinson Jun 07 '16 at 16:46
  • `std::set` stores its elements sorted, BTW. Sorting `std::list` is probably not very good idea. Is there any special reason you allocate `cl1` on the heap? – bipll Jun 07 '16 at 19:09
  • 1
    Ah, possibly a point we all were missing so far: `operator<`, `operator>`, `operator<=`, `operator>=` all are case sensitive! So, as upper case letters have a smaller ascii codes than lower case letters, this will result in "Zebra" being sorted *before* "ant"... In this case, even `std::set` or `std::sort` won't help, unless you provide them your custom, case-insensitive comparator. – Aconcagua Jun 07 '16 at 19:25
  • Hi everyone i have been trying these solutions and I noticed something interesting when attempting to rebuild the file. Check the edits up top. Thanks for everyone's time I sincerely appreciate it. – StormsEdge Jun 08 '16 at 12:07
  • @StormsEdge Seems as if the program was still running while trying to rebuild. You'd need to stop your program before rebuilding it. – Aconcagua Jun 08 '16 at 12:41
  • @Aconcagua I tried closing out of eclipse to ensure it wasn't still running and when I attempt to reinitialize it (run it again) it gives me a warning that there are errors would I like to continue to run – StormsEdge Jun 08 '16 at 12:43
  • @StormsEdge Have you ever checked in task manager, if the exe is still running? If a previous build failed for *any* reason, this is the warning you get. Rebuild the application to get rid of it. If you confirm the warning to go on, you will be using the result of the last successful build, which most probably is outdated (in respect to the state of your code). – Aconcagua Jun 08 '16 at 12:49
  • @Aconcagua do you know what the exe file would look like? I just closed all the files I could find in the task manager that were associated with eclipse and came back into the application. Attempted to build and received the same error Permission denied. I'm also receiving in the pop up "Errors exist in the active configuration of project "Program18ContactList". Proceed with launch? – StormsEdge Jun 08 '16 at 13:25
  • 1
    @StormsEdge You do not need to close your IDE, you need to shutdown the process of your executable, which is named `Program18ContactList.exe` as your error message you provided with the latest edit shows. Probably there is yet an instance of running, so Windows keeps a file lock on it and the linker cannot access it, just what the error message says. As long as this situation persists, you won't be able to recompile your program, and thus you will continue getting the error message ("exists! proceed?"). – Aconcagua Jun 08 '16 at 15:36
  • @Aconcagua That fixed the problem! Thanks so much. I found an executable running and that fixed everything. It appears that it was deferring to the old version of the code (like you said) rather than recompiling the new project. – StormsEdge Jun 08 '16 at 15:45

1 Answers1

1

Your list seems to be working fine so far. Be aware, however, that calling addToHead would break sorting. Of course, you did not do so, but you offered an interface capable to break the intended functionality of your list (which is of being sorted). Better leave it out entirely.

The problem you most probably encountered is that operator>= is case sensitive, so "Zebra" will not be considered larger than or equal to "ant" (if(curr->name >= newNode->name) break;) and thus "ant" will be sorted behind "Zebra", as ascii 'Z' (90) is smaller than ascii 'a' (97).

You can provide our own operator (must be at least declared at latest before the definition of insert):

bool operator>=(std::string const& x, std::string const& y)
{
    std::string::iterator ix = x.begin();
    std::string::iterator iy = y.begin();
    while(ix != x.end() && iy != y.end())
    {
        if(toupper(*ix) > toupper(*iy))
            return true;
        if(toupper(*ix) < toupper(*iy))
            return false;
    }
    // both strings are equal so far...
    // at least one string is at its end
    //
    // if iy is, then y is not longer than x and thus
    // lexicographically not larger than x (so x is larger or equal)
    return iy == y.end();
}

This works, as, luckily, the operators for std::string actually are templates, so this operator will have precedence in overload resolution.

Just a side note: This definitely works fine for ascii, and works for most languages if using single-byte encodings. There are issues, however, with e. g. Turkish, and you might run into problems if using utf-8. Probably beyond scope now, but you might remember in the future...

I personally tend to consider "Alpha" being larger than "alpha", "bEta" being larger than "beTA", ... (opposite as with ascii code!) - but really, just a matter of taste. Still, you could enforce such sorting by replaceing return iy == y.end(); with:

if(iy != y.end())
    return false;
if(ix != x.end())
    return true;
return x <= y; // opposite, see above... if you agree with ascii, use >=, of course
Aconcagua
  • 24,880
  • 4
  • 34
  • 59