C++ Class Definition

[CJPowell27]

Hey guys, I've been working on my homework all day today trying to debug my code and I have gone

over it multiple times trying to see what is causing the  issue.  However, there is something wrong with my 

classes that is causing everything to do with it to not be defined.  I have been staring and changing some 

of the code but to no avail.  Could someone point out what I am overlooking as I have no idea what is

causing my problem.  I'm not asking you to solve it, but it would help to simply point out what line i should

change  Here are my files: header.h, main.cpp, functions.cpp.  Here is a screenshot of the error codes: 

[PlutoNZL]

I'm on my phone at the moment so I can't help too much. 

 

One of the issues you are hitting is that you are referencing a class (note) that hasn't been defined. In C++ you need to define a class before you can use it. You're node class is referencing note which is defined later in the file. You need to define note earlier than node in header.h. 

[CJPowell27]

1 hour ago, PlutoNZL said:

I'm on my phone at the moment so I can't help too much. 

 

One of the issues you are hitting is that you are referencing a class (note) that hasn't been defined. In C++ you need to define a class before you can use it. You're node class is referencing note which is defined later in the file. You need to define note earlier than node in header.h. 

Oh geez that totally makes sense! Basically the whole concept of prototypes and functions ordering, thank you!

[CJPowell27]

Current errors after moving that around.  I know the error for line 37 will be easy as i just have to implement the function.  But shouldn't my head already be declared?

[PlutoNZL]

4 hours ago, CJPowell27 said:

Current errors after moving that around.  I know the error for line 37 will be easy as i just have to implement the function.  But shouldn't my head already be declared?

I quickly looked through all your files in the OP and couldn't see head declared anywhere. Where did you declare it? 

[Unimportant]

note::note
{
    char keyword[20];
    int chapter = 0;
    char notes[100];
}

If this is supposed to be note's constructor, it has several problems.

First it should be note::note()

Second, keyword and notes are automatic variables that will be created when entering the constructor and destroyed again when it's left. They have nothing to do with the pointers *keyword and *notes that you declared in the header file. Since there is no reason to create these 2 objects dynamically i suggest you just declare the arrays in the header.

 

 
private:
        char keyword[20];
        int chapter;
        char notes[100];

 

Then, in function   int table::insert(char * key_value, const note & toAdd)

    temp = new node;
    if(!temp -> entry)

You seem to be testing if the memory allocation failed by looking for temp to be a nullptr. C++'s new operator does not return a nullptr upon failure. It throws a std::bad_alloc.

try
{
	temp = new node;
}
catch(std::bad_alloc&)
{
	//No need to delete as allocation failed.
 	return 0;
}
    

You need to include header <new> for std::bad_alloc.

 

For the next error:

temp -> next = hash_table[i];

I don't know what you're trying to do but i is undeclared in that scope. You never created a variable called i, neighter locally or as a class member.

 

 int index = /*Hash function*/;

That's comment where's there supposed to be code. Perhaps this should be a call to int table::hash_function(char * key)const that you forgot to change?

 

    node * current = hash_table[index];
    while(current && !success)
    {
        success = current -> retrieve(keySearch, found);
        current = current -> next;
    }

Again, not sure what you're trying to do but C++'s -> operator is meant for accessing members trough a pointer, so  current->retrieve(keySearch,  found); is trying to call node::retrieve, which does not exist.

 

In void table::writeout(), same error as before:

  node * current = head;

There is no variable called head in this scope, neighter locally or a class member.

 

out << current -> note;

Same as before:  node::note does not exist.

 

In function main:

toAdd.create(title, toAdd);

toAdd.create(title, toAdd);

There is nothing called title in that scope.

 

On 2 more occasions in main you again have comment where there should be code:

myTable.insert(/*data*/,toAdd);

 

[CJPowell27]

19 hours ago, Unimportant said:

note::note
{
    char keyword[20];
    int chapter = 0;
    char notes[100];
}

1.If this is supposed to be note's constructor, it has several problems.

First it should be note::note()

Second, keyword and notes are automatic variables that will be created when entering the constructor and destroyed again when it's left. They have nothing to do with the pointers *keyword and *notes that you declared in the header file. Since there is no reason to create these 2 objects dynamically i suggest you just declare the arrays in the header.

 

 
private:
        char keyword[20];
        int chapter;
        char notes[100];

 

 

2.

 int index = /*Hash function*/;

That's comment where's there supposed to be code. Perhaps this should be a call to int table::hash_function(char * key)const that you forgot to change?

3.

    node * current = hash_table[index];
    while(current && !success)
    {
        success = current -> retrieve(keySearch, found);
        current = current -> next;
    }

Again, not sure what you're trying to do but C++'s -> operator is meant for accessing members trough a pointer, so  current->retrieve(keySearch,  found); is trying to call node::retrieve, which does not exist.

 

 

I numbered the stuff that I havent solved in the quote to reference them easily.  

For number 1, one of the requirements for my homework assignment is to dynamically

 allocate all arrays and honestly I never learned how to do that in the previous class

(probably missed that day or something).  For number two, I do want to call that hash

function but I am not entirely sure how to.For number 3 how should I call retrieve? would

I want to use something like entry.retrieve?

[Unimportant]

1)  I don't know what you've been taught at school yet so I'll explain 2 ways of dynamic allocation, the easy way (wrong, unsafe, but often taught at schools) and the right, slightly more complex,  way.

 

You had the header right for the first way:

private:
        char * keyword;
        int chapter;
        char * notes;

Then, the the constructor:

note::note()
{
    keyword = new char[20];
    int chapter = 0;
    notes = new char[100];
}

This dynamically allocates 20 chars for keyword and 100 for notes. Because you now allocated memory, you have to free it, so you need a destructor.

note::~note()
{
	delete [] keyword.
	delete [] notes;
}

Note the '[]' after delete. Use this when deleting a array created with new[], and use delete without [] for deleting objects allocated with new without []. They should always match, doing it wrong leads to undefined behavior.

 

There is something called 'the rule of three' in C++ that says that if your class manages a resource (which dynamic memory clearly is) you should also provide a copy constructor and a assignment operator. The reason is simple - If you were to make a copy of your class as it is, the default copy constructor would simply copy the pointers and you'd have 2 class instances working with the same memory. The copy constructor should allocate it's own memory and copy the data. A full explanation would take us too far so i suggest you research "c++ the rule of three" on google or in books, or ask your professor.

 

Since c++ 11 we also have move semantics to research if you're really interested.

 

The constructor example above is actually unsafe code however. It breaks an important rule that states that a class should only handle 1 resource. Due to this it's for example not exception safe. consider that the allocation of 'notes' fails, and new[] throws a std::bad_alloc exception. Since the constructor never completely finished, the class is regarded as never beeing constructed and thus, the destructor is never called. The memory allocated for 'keyword' is now leaked.

 

The solution is to use RAII to wrap the pointers and make them behave like automatic variables on the stack. ( https://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization )

 

C++11 offers std::unique_ptr for this purpose. It's a wrapper class that holds a pointer inside, only one class instance can own the pointer (If you copy it, the original holder loses the pointer) and the memory will be de-allocated automatically when the unique_ptr is destroyed.

private:

	std::unique_ptr<char[]> keyword;
        int chapter;
        std::unique_ptr<char[]> notes;  
    

The constructor changes as follows:

note::note()
{
	keyword.reset(new char[20]);
    	int chapter = 0;
	notes.reset(new char[100]);
}

And now you no longer need a destructor, because the unique_ptr's will clean themselves up when the stack is unwound. Google std::unique_ptr to learn more on how to use them. (you still need copy constructor and assignment operator tough).

 

I'll look to the other 2 things when i find more time.

[CJPowell27]

12 hours ago, Unimportant said:

 

Wow you just taught me that better than the tutors at our CS lab could lol.  

Basically all that I have left is not declared in this scope as I have no idea how to 

have the functions have access to what it needs.  All the codes have to do with my

hash_table function. I'm also not sure how or where I should declare found.

[Unimportant]

The code you originally posted @ pastebin no longer matches the error messages in your last post as it was changed. Can you update the code ?

[CJPowell27]

On 5/13/2016 at 3:31 PM, Unimportant said:

The code you originally posted @ pastebin no longer matches the error messages in your last post as it was changed. Can you update the code ?

Oops will do in the morning...it's 12:30 am currently

[CJPowell27]

functions

header

main

[Unimportant]

Destructor 'table::~table()' only needs to clean up the allocated memory, so the line:

node * head = hash_function[i];

looks like it's unnecessary to me.

 

'hash_table_size' is also never declared, you probably mean 'tableSize' ?

 

The destructor might look something like this:

for (int i = 0; i < tableSize; i++)
{
	//Run trough all pointers in hash_table and delete each node who's
	//pointer which is not NULL.
	if (hash_table[i])
	{
		delete hash_table[i];
		hash_table[i] = NULL;	//nullptr is preferred since C++11.
	}
}

//delete memory allocated for hash_table itself.
delete [] hash_table;

Note that in real code we would use a std::vector or std::array for 'hash_table' and not risk lots of bugs and leaks by managing this manually.

 

In 'table::insert' :

node * temp = new node;
temp = new node;

if(!temp)
{
  	delete temp;
  	return 0;
}

declares a node pointer called temp and allocates memory for it. In the very next line you again allocate memory and overwrite the pointer, the first allocated block is lost forever - memory leak.

new does not return a nullptr when allocation fails, it throws std::bad_alloc, so the 'if' line is completely pointless.

node * temp;

try
{
	node * Trytemp = new node;
	temp = Trytemp; //Copy to temp because Trytemp's scope is limited to the try block.
}
catch(std::bad_alloc&)
{
  	return 0;
}

Although it's a little inconsistent to test if this allocation fails and not all the other allocations.

 

In 'table::retrieve':

int index = hash_function(key);

'key' is undeclared. Perhaps you mean 'keySearch' ?

 

In 'note::retrieve':

int index = hash_function(key);
node * current = hash_table[index];

Again 'key' is undeclared but also, 'hash_function' and 'hash_table' are members of class 'table', you can't use them from within class 'note' without a pointer or reference to an instance of class 'table'. However, such code immediately betrays  something is wrong with your class hierarchy. Why is one of your classes trying to access the private 'guts' of another class - preventing this is exactly what encapsulation in OOP is meant for. You might need to think a bit more about what each class's core responsibilities should be and how they interact.

 

 

[CJPowell27]

1 hour ago, Unimportant said:

Destructor 'table::~table()' only needs to clean up the allocated memory, so the line:

node * head = hash_function[i];

looks like it's unnecessary to me.

 

'hash_table_size' is also never declared, you probably mean 'tableSize' ?

 

The destructor might look something like this:

for (int i = 0; i < tableSize; i++)
{
	//Run trough all pointers in hash_table and delete each node who's
	//pointer which is not NULL.
	if (hash_table[i])
	{
		delete hash_table[i];
		hash_table[i] = NULL;	//nullptr is preferred since C++11.
	}
}

//delete memory allocated for hash_table itself.
delete [] hash_table;

Note that in real code we would use a std::vector or std::array for 'hash_table' and not risk lots of bugs and leaks by managing this manually.

 

In 'table::insert' :

node * temp = new node;
temp = new node;

if(!temp)
{
  	delete temp;
  	return 0;
}

declares a node pointer called temp and allocates memory for it. In the very next line you again allocate memory and overwrite the pointer, the first allocated block is lost forever - memory leak.

new does not return a nullptr when allocation fails, it throws std::bad_alloc, so the 'if' line is completely pointless.

node * temp;

try
{
	node * Trytemp = new node;
	temp = Trytemp; //Copy to temp because Trytemp's scope is limited to the try block.
}
catch(std::bad_alloc&)
{
  	return 0;
}

Although it's a little inconsistent to test if this allocation fails and not all the other allocations.

 

In 'table::retrieve':

int index = hash_function(key);

'key' is undeclared. Perhaps you mean 'keySearch' ?

 

In 'note::retrieve':

int index = hash_function(key);
node * current = hash_table[index];

Again 'key' is undeclared but also, 'hash_function' and 'hash_table' are members of class 'table', you can't use them from within class 'note' without a pointer or reference to an instance of class 'table'. However, such code immediately betrays  something is wrong with your class hierarchy. Why is one of your classes trying to access the private 'guts' of another class - preventing this is exactly what encapsulation in OOP is meant for. You might need to think a bit more about what each class's core responsibilities should be and how they interact.

 

 

for note::retrieve I was trying to use the char * key that the table::hash_function should I move the hash function to the note:: class? Or how would I write the code to have a pointer to the hash_function? Also how should I declare found?