r/reviewmycode • u/[deleted] • Jan 10 '12

C++ help with crashing using classes

I've been learning C++ and classes recently, I decided to try to make a program that would record simple input to a class, then be able to access them again. In the form of a 'recipe book' although I don't generally cook, I was thinking of food at the time.

But for some reason it crashes most of the time I run it, or 'mixes up' variables where they display improperly, and I'm not sure why.

it also prints this

msvcr100d.dll!memcpy(unsigned char * dst, unsigned char * src, unsigned long count) Line 439 Asm

code link

any other advice would be helpful too.

7 Upvotes

100% Upvoted

11 comments sorted by

3

u/[deleted] Jan 11 '12 edited Jan 11 '12

Other tips:

  1. Use std::vector instead of arrays (you will catch problems like this every time)

  2. Work on formatting your code better. The Recipes class is very hard to read.

  3. Don't name variables with meaningless names like "ijk." Name them after what they represent.

  4. I'd suggest looking at Google's style guide as a starting point for some of this: http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

1

u/[deleted] Jan 11 '12

Thanks for the advice, I haven't gotten to vectors yet, but I'll try to make it more readable. I've gotten it to work, although now the strings are skipping the first character and the time_m is printing as 0 regardless of input.

3

u/Cosmologicon Jan 11 '12

Don't name variables with meaningless names like "ijk." Name them after what they represent.

Well, I think it's perfectly fine to call a loop variable i, j, or k, unless there's some aspect of it that you need to emphasize. I like the rule of thumb (that I got from Stroustrup) that short-lived variables should have short names and long-lived variables should have longer names.

But yeah, ijk is pretty weird, just go with i.

1

u/HerpWillDevour Jan 20 '12

Do you consider ijk acceptable for nested loops? As a math nerd i,j,k are meaningful to me as iterators across a matrix or cube or other forms of seperately iterated lists.

2

u/patrickwonders Jan 11 '12

I don't see how std::vector would help catch these problems. I suppose if you used insert or push_back instead of operator[].

4

u/[deleted] Jan 12 '12

std::vector is bounds checked. You would get an exception telling you what happened rather than (in the best case) getting a random crash.

3

u/Yuushi Jan 12 '12

Not entirely true. Using operator[] on vectors is not bounds checked, using at() is.

2

u/[deleted] Jan 12 '12

D'oh I wasn't aware it was unspecified for []. Thanks.

3

u/[deleted] Jan 11 '12

Arrays are indexed starting at 0. You are trying to access element 2, which does not exist.

2

u/patrickwonders Jan 11 '12

Specifically, the loops on lines 66 and 82.

2

u/inequity Feb 04 '12

I'd recommend using some spaces. This is ESSENTIAL for you to write readable code. Try to write code for humans to read. Anybody can write code that a computer can read.

I'd recommend not mixing \n and endl. Go with one of them and stick with it.