Garbage Result! C++ LinkedList

Question

Given the code in Main:

// main.cpp
wineries->insert(winery("Lopez Island Vinyard", "San Juan Islands", 7, 95));

Two things will happen: The winery ctor is invoked where I have intialized the winery private members:

//winery.cpp
winery::winery(const char * const name, const char * const location,
               const int acres, const int rating)
  : name( new char[strlen(name)+1] )
  , location( new char[strlen(location)+1] )
  , acres( 0 ), rating( 0 )
{

}

When it is finished the result of the 'this' pointer has garbage value. Why is this?? I am not intializing incorrectly..

Second thing that happens, After the winery ctor dies we go to the list::insert( const winery &winery ) function:

void list::insert(const winery& winery)
{
    node *NodePtr = new node( winery );
         // NodePtr->item has the garbage.
    NodePtr->item = winery;
}

list::node::node( const winery& winery )
{
   // This works because I have a default ctor for the winery object
   // and ONLY for that reason...
  //How can I use the node ctor without having to use a default ctor for the winery class?
}

Why am I getting garbage as a result of the values passed to the winery ctor? The winery public member functions are as follows, where name, location, acres, and rating are all private members to the winery class.

winery::winery()
{
    // do nothing dfault ctor 
         // only here so I can add the &winery to the node ctor..
}

winery::~winery()
{
    delete location;
    delete name;
    // your code here
}

const char * const winery::getName() const
{   
    //winery *wine_t = new winery();
    const char cName[5] = "four";
    // just to see if It still gives garbage..
    return cName
}

const char * const winery::getLocation() const
{
// return one of winery's private members.
// It WILL Crash when this function is called.
// THAT might be the issue with **garbage values**
    return location;
}

Without these functions have paramters, it makes it difficult to transfer the attributes over to a wineryPtr object and then it would be logical to add the entire winery object to the linkedlist......

// list.h
#ifndef _LIST_
#define _LIST_

#include <ostream>
#include "winery.h"

using namespace std;

class list
{
public:
    list(void);    // constructor
    virtual ~list(void); // destructor
    ...
         void insert(const winery& winery);
    winery * const find(const char * const name) const;


private:
    struct node
    {
     node(const winery& winery);  // constructor
     winery item;
     node * nextByName;
     node * nextByRating;
    };

    node * headByName;
    node * headByRating;
};

#endif // _LIST_

YEAH! So my questions are a little scatterd, and I hope someone out their has the time to help!

Answer 1

Probably the reason is you are lacking a copy-constructor and operator= in your winery class. When you insert it into the list, the name and location pointers are copied across, but the strings they are pointing to are shared. Then the instance you copied it from goes out of scope, and the instances are deleted.

In your debugger, place a breakpoint in the constructor where you create the name and location, and then also in the destructor. You will see that the number of allocations is not matched by the number of deallocations.

Answer 2

Alright. Somehow in my typing I can see a little more clearly.

This is what i meant:

winery::winery(const char * const name, const char * const location, const int acres, const int rating): name( new char[strlen(name)+1] ), location( new char[strlen(location)+1] ), acres( 0 ), rating( 0 )
{
    strcpy_s( this->name, MAXNAME_SZ, name );
    strcpy_s( this->location, MAXNAME_SZ, location );
    this->acres  = acres;
    this->rating = rating;
}

where, MAXNAME_SZ is a const int of 75.

Answer 3

Your implementation of winery constructor does not look right for me:

//winery.cpp
winery::winery(const char * const name, const char * const location,
               const int acres, const int rating)
  : name( new char[strlen(name)+1] )
  , location( new char[strlen(location)+1] )
  , acres( 0 ), rating( 0 )
{
}

First, you just allocated memory for name and location member variables, but did not srtcpy the content from input parameters.

Second, you initiated acres and rating member variables with zeroes. Is it your intention?

And finally, you need copy constuctor and assignment operator!

Answer 4

The cause of a lot of the issues is probably the fact that your node contains a winery object by value and your winery default constructor is not assigning NULL to location and name. Your insert operation is creating a winery object using the default constructor when it allocates the node object, then the object is overwritten when the NodePtr->item assignment occurs. This causes the assignment operator to be invoked. The assignment operator cannot be implemented correctly without the default constructor doing the right thing.

I'm making some assumptions here since I don't know what list::node::node(const winery&), winery::operator=(const winery&), or winery::winery(const winery&) look like but I can't imagine that they can be implemented correctly without having the winery members set to a known value by default.

I would carefully walk through the insert() call that you mentioned until you completely understand what operations are being called. Consider the following program.

#include <iostream>

struct node {
  node() { std::cout << "default constructor" << std::endl; }
  node(int) { std::cout << "int constructor" << std::endl; }
  node(node const&) { std::cout << "copy constructor" << std::endl; }
  ~node() { std::cout << "destructor" << std::endl; }
  node& operator=(node const&) { std::cout << "assignment" << std::endl; }
};

void
insert(node const& a) {
  node b(a);
  node c;
  b = c;
}

int
main() {
  insert(node(1));
  return 0;
}

Run it and see if you can follow why the output is:

int constructor
copy constructor
default constructor
assignment
destructor
destructor
destructor

You problem is that your default constructor, copy constructor, and assignment operators are not doing the right thing. Hopefully this will start you down the right path.