macOS C++ linkedlist problem for a class

[jordanste]

hey guys, im taking an introductory c++ class right now at a local college and my final is to build a linked list application that allows you to build a list of records, search them, delete a record, and add a record.

i have most of it finished, im just having a problem with adding a new record and im getting a bus error, not sure why.

im going to post the code below (in full) maybe someone can help point out the flaw for me. its alot for a forum post i know, about 250 lines... but the issue should be found in the insertNode() function.
I hope someone can help. thanks so much!

**code removed

 

[lee1210]

I am going to take a look, but first and foremost:
1) I saw struct definitions at the top of the file (file, singular). Were classes not introduced in this course?
2) As noted above, there is only one source file, and no headers. Were you told to solve this problem in this manner, or was this your choice? If the latter, I recommend rethinking this decision.
3) Were you told to use a singly-linked list instead of a doubly-linked list, or was this your choice? If the latter, why did you choose singly-linked (I'm not asserting this was the wrong decision, just curious if you considered the pros and cons)?
4) I haven't read the whole thing, but why doesn't a Node contain a Person? It seems to have 3/5ths of the fields of Person duplicated.
5) I will be perfectly honest, I am not sure about the ins and outs of the C++ new (and on the other end, delete) operator, but does it work the same with structs as classes/objects?

I am sure there will be more questions as I go through the code, but those are what jumped out during a cursory look.

-Lee

Edit:
Good news: This source compiles without warnings with g++.
Bad news: I do not have data.dat, and while I am sure I can figure out what its format will be parsed by this program, it would be nice if it were included.
Edit 2: One last question: Were you instructed to put the * in declarations next to the datatype instead of the variable name, or was this your decision? If the latter, I would recommend choosing the other option for clarity's sake.

 

[lee1210]

I am not a fan of double-posting, but didn't feel like continuing to edit one post, in case someone were to be monitoring the thread.

More questions:
1) Is the indentation and bracing style your own, or enforced by your professor? Some of this is just a matter of personal preference, but an opening brace on its own line infuriates me.
2) Is for the only loop control structure that has been introduced? It is very unusual to see an if with no initialization section or increment in the post-iteration section. If you want to run until something is or is not true, that's normally expressed with a while loop. If you want to do something at least once, until something is or is not true, a do-while loop is the traditional means of doing so. The particular section of the code i am referring to seems to work, it just looked strange.

-Lee

 

[lee1210]

I can reproduce the crash. It appears that in the section of your switch statement for adding a record, a firstname is read (and only a first name). A search is performed to see if this firstname is found (the most unique thing you are storing is SSN, that seems better as a search key, but you may have been asked to treat first name as unique), and if not, insertNode is called.

In the scope within which this is called (main), this Node * is declared and initialized to NULL (though with a strange syntax, casing 0 to a Node * instead of using the #defined NULL, which is clearer when dealing with pointers). Nothing else modified newNode, so it is NULL when passed to insertNode. When an attempt is made to access fname of newNode, the value offset for fname is calculated (0), and added to the base of the structure, which is 0/NULL, so you attempt to access the memory at address 0, leading to an access violation.

I would suggest, if the first name is not found, asking for the rest of the information needed for a person, getting a new node, and filling it in with this information. Then call insertNode.

I am making no warranties as to the functionality of the rest of the code, but this should take care of the error causing the crash right now.

-Lee

Edit: Just another question:
Why read the age and salary if you intend to do nothing with them? Why have a Person struct with these things if you are only going to use it for input, but aren't actually going to store the Person (or all of its fields)?

 

[jordanste]

hey, thanks for getting back so quickly.

First of all classes were not introduced in this course, the instructor said he wanted us to use structs for now.

The instructor basically gave us code for displayList() and buildList(), he gave us the structs, and he told us kind of how he wanted the menu to work with the switch and everything. he then said build searchNode(), deleteNode(), and addNode().

the bracing style is roughly my own, he doesnt care that much he asks us to be consistent. He always places opening braces on their own line so I just picked it up.

for is the loop that has been used for everything in this course, he hasnt really gone over while or dowhile.

I realize alot of this code may not be written in the best way possible, or even in a necessarily good way. This isnt production code, its just for a course.

I honestly barely grasp the concepts of pointers and everything right now, so i really just want to know why im getting a bus error when i try to use insertNode() by using menu option (4). However when insertNode() is called inside of buildList() there is no error.

the data file is just a list of first and last names, social security numbers, salaries, and employee ids. the names are all cartoon characters... hah.


**code removed

 

[lee1210]

Read over the rest of my posts for more information about the error.

"This isnt production code, its just for a course."

If you don't learn to write code good enough for production in a course, where will you learn it?

Also, Bugs Bunny is probably the most recognizable character on that whole list, but has the lowest salary. He must have negotiated a poor contract without consideration for inflation, etc.

-Lee

 

[toddburch]

The problem is that in the case 4: routine, when the entry is not found, you are failing to "new" a newNode, and therefore, are passing a NULL newNode to insertNode.

 

[lee1210]

The problem is that in the case 4: routine, when the entry is not found, you are failing to "new" a newNode, and therefore, are passing a NULL newNode to insertNode.

I guess that's a little more concise than:

In the scope within which this is called (main), this Node * is declared and initialized to NULL (though with a strange syntax, casing 0 to a Node * instead of using the #defined NULL, which is clearer when dealing with pointers). Nothing else modified newNode, so it is NULL when passed to insertNode. When an attempt is made to access fname of newNode, the value offset for fname is calculated (0), and added to the base of the structure, which is 0/NULL, so you attempt to access the memory at address 0, leading to an access violation.

=)

-Lee

 

[toddburch]

lol. I started reading that, and got confused as I hadn't studied the code yet.

 

[lazydog]

Hi

Now that you know what your bug is I thought it might be useful to make a comment on your comments. You might get better marks if you add more meaningful comments to your code other than

/* END searchNode */

/* END for */

which don't really say anything.

For example, I would comment insertNode() something like this:-

/**
 * This function inserts a 'floating' node into an existing linked list
 * of nodes begining with head.
 * Note, the head of the list may change value after calling this function.
 * 
 * Returns : the head node of the list
 */
Node* insertNode( Node* head, Node* newNode, char* fname )
{
    Node* current=head ;
    Node* prior=NULL ;
    
    //
    // Find the appropriate place to insert the new node.
    // The new node is to be inserted between prior and current.
    //
    while(  current && strncmp( current->fname,newNode > fname, NAME_SIZE ) < 0 )
    {
       prior=current ;
       current=current->next ;
    }

    //
    // Insert the new node in between prior and current
    // by linking the prior node to the new node and the new node
    // to the current node.
    // 
    newNode->next=current ;
    
    if( ! prior )
        //
        // There was no prior node, so the new node will be the head.
        //
        head=newNode ;
    else
       //
       // There was a prior node so link it to the new node.
       //
       prior->next=newNode ;
 
    //
    // Return the head of the list because it may have changed to newNode.
    //
    return head ;
}

Apart from being good coding practise it will show that you understand what's going on.

b e n

 

[jordanste]

hey guys, i did indeed find the bug in the case 4 option of the switch. thanks.

also, concerning comments, we are instructed to remove any comments from our code upon submission, our instructor then inserts his own comments while grading our code. i simply have made a habit of commenting all closing braces so that I do not overlap them. im coding in text edit. it doesnt flash the opening brace like alot of text editors do. but i will have to remove all comments from my code before i submit it. thanks again!

 

[lee1210]

I hope my concern is without merit, but I am very concerned about the caliber of the instruction of this course. It's C++, but the only non-C stuff I saw was some iostream things, which would easily be replaced with C I/O. The code you said the instructor wrote for build_list, the structs, etc. doesn't really make sense (not having a Person in a Node sort of blows my mind). Asking you to strip comments? That to me means "I don't care if you use comments", which is awful in an introductory class, when you are forming your coding style, etc. Only teaching for loops, and not while, etc. seems very sloppy.

You can obviously get code running, but I would be worried about future style, etc.

I don't mean to discourage you, and perhaps if there is a follow-up course this will all be covered, but I wouldn't want you to develop bad style or sloppy use of control structures this early on.

-Lee

 

[bearda]

im coding in text edit. it doesnt flash the opening brace like alot of text editors do. but i will have to remove all comments from my code before i submit it. thanks again!

I would strongly recommend you start using an editor that's more programmer-oriented or a full IDE If you're on a Mac and are targeting it at gcc/g++ XCode is pretty decent, and free. When you start running into problems the debugger can be a lifesaver, as well.

I did a similar thing while I was in college, using nano to edit code through a remote terminal connection. In hindsight I regret not using a lot of the tools that were available at the time.

 

[gnasher729]

5) I will be perfectly honest, I am not sure about the ins and outs of the C++ new (and on the other end, delete) operator, but does it work the same with structs as classes/objects?

Yes. In C++, struct and class are practically the same thing. The only difference is that if you don't use "private", "public" and "protected", struct members are public by default and class members are private by default.

Of course you should use them in quite different ways.