PDA

View Full Version : Problem calling a function in a class




EdoDodo
Oct 18, 2009, 01:45 PM
Problem solved, thank you!

--------------------------------
Could someone help me out with one of my projects?

I'm trying to write a small Objective-C program that takes the users guesses of what a code made up of colors could be and tells him how many of them he got right and how many colors are in the code but not in the right place.

It's only partly done, but I get a problem when I try to call the printHistory method of the Code class I have created. The printHistory method should print an array containing (in theory) all of the user's guesses so far, in fact I have hard-coded some values in to test it. However, when I get to the part of the program where that (or even when I try a simple method in the class that only increments a count of how many guesses the user has made), I get a bad access error.

Could anyone tell me what I'm doing wrong, please? You can check out the project here:
http://www.2shared.com/file/8519245/1f20632c/Crack_the_Code.html

PS: Sorry for my terrible coding habits, I was going to clean up the code later once it was working properly, and wasn't planning to show it to anyone just yet :P.



lee1210
Oct 18, 2009, 02:15 PM
I'm taking a look now. I'm not getting any errors running the code, but there are definitely some oddities here. You have implementation in a .h file, and prototypes in the .m, which is exactly opposite of the normal way of doing things.

You mention that you plan to "clean the code up later". This is a bad way of approaching writing a program. In the meantime you (and now anyone that wants to help you) has to slog through what you consider poorly formatted, documented, etc. code to find a problem. If you always write code you consider "good", than you're never having to read bad code trying to find a bug.

What exactly are you doing that's generating the error?

-Lee

lee1210
Oct 18, 2009, 02:29 PM
In the scope of your switch statement you defined a new Code * called theCode, which masked the variable theCode declared outside of your while with scope in main. Unless you chose "new" from your menu, theCode with switch scope is uninitialized, so the messages passed are simply discarded. Change:

Code *theCode = [[Code alloc] init];

to

theCode = [[Code alloc] init];


in the 'n' case of your switch.

-Lee

EdoDodo
Oct 19, 2009, 12:44 AM
Thanks, I didn't realize that releasing the object kept the theCode variable floating about with a pointer to an inexisting object. And whoops for switching around the .m and .h, fixed that too, thanks. Yeah I just realized how horrible the code was now that I started trying to debug it :P, I'll try and write something a bit cleaner in the future :P.

lee1210
Oct 19, 2009, 08:41 AM
While releasing does leave the pointer "hanging" (what else could happen? You can't get rid of a local variable), that wasn't the real issue here. The real issue was that you had two theCode's, one with scope in main and one with scope in your switch statement. You allocated a Code object and assigned the pointer to theCode with scope in main, but then masked that variable with the version in your switch scope, so in each of your case statements in your switch, when you were passing a message to theCode, you were referencing the switch-scoped copy, so unless you'd chosen 'new' from the menu, which would assign a new instance of a Code object, you'd be passing messages to a nil object pointer.

I just found the compiler option to get warnings to show up when you do some things like this. Not at a mac, but there should be an "other compiler flags" option or something like that where you can add -Wshadow. Then you'll get a warning if you mask (or "shadow" is i guess what some call it) a variable in a higher scope w/ a new declaration. It would be of value to add this so you catch this sort of thing in the future.

-Lee

EdoDodo
Oct 19, 2009, 09:26 AM
While releasing does leave the pointer "hanging" (what else could happen? You can't get rid of a local variable), that wasn't the real issue here. The real issue was that you had two theCode's, one with scope in main and one with scope in your switch statement. You allocated a Code object and assigned the pointer to theCode with scope in main, but then masked that variable with the version in your switch scope, so in each of your case statements in your switch, when you were passing a message to theCode, you were referencing the switch-scoped copy, so unless you'd chosen 'new' from the menu, which would assign a new instance of a Code object, you'd be passing messages to a nil object pointer.

I just found the compiler option to get warnings to show up when you do some things like this. Not at a mac, but there should be an "other compiler flags" option or something like that where you can add -Wshadow. Then you'll get a warning if you mask (or "shadow" is i guess what some call it) a variable in a higher scope w/ a new declaration. It would be of value to add this so you catch this sort of thing in the future.

-Lee

Thanks, I have enabled the option to stop myself from making this mistake again. My mistake was based on my misunderstanding that when I did [theCode release] the Code *theCode (the variable holding the pointer to the item) would disappear along with the object and I would have to recreate it, thanks for clarifying that for me. Also indeed I was making a mistake with thew scope of the variable there, I guess I'll have to re-read a bit on the scope of variables.