3

Ok, this is for homework about hashtables, but this is the simple stuff I thought I was able to do from earlier classes, and I'm tearing my hair out. The professor is not being responsive enough, so I thought I'd try here.

We have a hashtable of stock objects.The stock objects are created like so:

stock("IBM", "International Business Machines", 2573, date(date::MAY, 23, 1967))

my constructor looks like:

stock::stock(char const * const symbol, char const * const name, int sharePrice, date priceDate): m_symbol(NULL), m_name(NULL), sharePrice(sharePrice), dateOfPrice(priceDate)
{    
setSymbol(symbol);
setName(name);
}

and setSymbol looks like this: (setName is indentical):

void stock::setSymbol(const char* symbol)  
{  
if (m_symbol)  
    delete [] m_symbol;  
m_symbol = new char[strlen(symbol)+1];  
strcpy(m_symbol,symbol);  
}

and it refuses to allocate on the line

m_symbol = new char[strlen(symbol)+1];

with a std::bad_alloc. name and symbol are declared

char * m_name;  
char * m_symbol;

It's definitely strlen() that is going astray. And it doesn't seem to happen every time. 

cout << symbol << strlen(symbol);

returns IBM correctly, then crashes

Improve this question
11
  • name and symbol are class variables, as well as the function parameters in the constructor and the setSymbol method. Maybe a bit confusing. Commented May 10, 2010 at 17:02
  • 1
    Do a std::cout << strlen(symbol) before the line that fails. Check that the string length value makes sense. Commented May 10, 2010 at 17:04
  • 1
    You should adopt a naming convention for your member variables. Two popular ones are memberVariable_ and m_memberVariable. Such a convention will make it easier to distinguish parameter names and member variable names. Commented May 10, 2010 at 17:06
  • 1
    Some pointer manipulation error might be causing the terminating '\0\' after "IBM" to get overwritten, resulting in a bogus strlen value. Commented May 10, 2010 at 18:04
  • 1
    Are you defining your own copy constructor and assignment operator? If you don't, the default ones generated by the compiler will perform shallow copies of your member variables (this is not what you want). See en.wikipedia.org/wiki/Object_copy, stackoverflow.com/questions/184710/…. Commented May 10, 2010 at 22:19

4 Answers 4

Reset to default
1

As this is tagged C++ can you use std::string instead of doing all the pointer maintenance yourself on char*?

std::string name;
std::string symbol

Then setSymbol becomes easy:

void stock::setSymbol(const char* symbol)  
{
    this->symbol = symbol;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

I do so hate it when C++ teachers demand that students use the C way of doing things, for no reason other than that the teacher is a C programmer wearing a hat that says ++. It is certainly valuable to know how strings work, but when it impedes understanding of other concepts, there is no sanity in demanding it.
@Quandrum: It might be easier to do the conversion just to see if it fixes the current bug, and then switch back.
1

There must be some problem with symbol parameter at the time you call

new char[strlen(symbol)+1];

and strlen return a huge length that C++ runtime is unable to allocate. If symbol is uninitialized char* pointer at the beginning this is fairly possible. It doesn't fail all the time, does it?

Improve this answer

4 Comments

I guess having the variables the same name is confusing. The symbol in strlen(symbol) is the function parameter, which points to the string "IBM" at this point. this->symbol is a class variable, which is a null pointer at this point. However, it doesn't fail all the time. You are correct.
A silly question (I would test If I had a compiler at hand but right now I don't) : is it possibel that the 'symbol' parameter and the 'symbol' instance variable somehow gets mixed up ? So at runtime strlen would run on a non-initialized string and give a bogus length ... Intuitively, you would think the compiler canno't be fooled, but with C++, you never know ;)
@phtrivier: Not a silly question. This problem is called variable shadowing: en.wikipedia.org/wiki/Variable_shadowing
@emile : thanks for the link... as you pointed elsewhere, that's the kind of errors you transparently get rid of by using different naming conventions for member / static / local variables ... but then choosing a convention opens the door to endless trolls ;)
0

I was able to run the code without problems on Cygwin, so I'd guess it's something implementation-dependent in distinguishing the parameter symbol from the member symbol.

You say yourself that it's confusing -- well do something about it!!! And may I suggest, never, ever again, naming a parameter the same as a local/member variable. (Not only does it eliminate confusion, you won't need to disambiguate the member variable with this->.)

Improve this answer

1 Comment

Mine runs without problems sometimes... even after I changed variable names to make it none confusing.
0

Thanks to everyone who offered help. I went over it with my professor, and unfortunately I was overflowing an array earlier and corrupting the heap, which was manifesting itself here.

This was a good conversation for me though. It helped me think through some things I had just been doing. So thanks again SO'ers

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.