views:

84

answers:

3

I'm trying to make a simple calculator application in cocoa. The program hangs when I click on one of my buttons. I think I've traced the problem to the part of my controller that adds a digit to the end of the number currently on the display:

- (void)updateNumber:(int)buttonClicked{
 *self.activeNumberPointer = *self.activeNumberPointer * 10 + buttonClicked;
 [outputField setFloatValue:*self.activeNumberPointer];
}

I used a pointer to the "activeNumber" in order to allow my program to tell which of the two operands I'm editing.

Any help appreciated, thanks.

(edit): My declaration and @property:

//CalculatorController.h
@interface CalculatorController : NSObject {
    IBOutlet NSTextField *outputField;
    float variable1, variable2;
    float *variable1Pointer, *variable2Pointer;
    float *activeNumberPointer;
}

float variable1 = 0;
float variable2 = 0;
float* variable1Pointer = &variable1;
float* variable2Pointer = &variable2;
float* activeNumberPointer = &variable1;

@property (readwrite) float variable1, variable2;
@property (readwrite) float *vairable1Pointer, *variable2Pointer;
@property (readwrite) float *activeNumberPointer;

...

Full XCode file available here: http://rapidshare.com/files/397664243/Calculator_2.zip (FYI: I actually used leftNumberValue and rightNumberValue instead of variable1 and variable2 in the project) Since I'm new to Objective-C and XCode, any general criticisms are welcome.

A: 

You don't need the * in front of self.activeNumberPointer or activeNumberPointer when you're using them, just when you define them. Try:

- (void)updateNumber:(int)buttonClicked{
 self.activeNumberPointer = self.activeNumberPointer * 10 + buttonClicked;
 [outputField setFloatValue:activeNumberPointer];
}
Mike Howard
Yes, but don't I need to add * before a pointer in order to get the value that it points to?
G.P. Burdell
A: 

Not clear on pointers in objective-C with simple variable types, and not totally clear why you're using pointers at all.

*self.activeNumberPointer = *self.activeNumberPointer * 10 + buttonClicked;

You are using the * operator (dereference) wrong on the left side of this expression. When doing assignment, you don't want to dereference like that.

http://www.cplusplus.com/doc/tutorial/pointers/

Thats a C++ reference, but a good start for pointer logic.

I would probably use an NSMutableArray or some construct like that (you want to store more than just two numbers right?), and use the integer index to point to the "current" number you're displaying on the screen.

Further, I don't know if its a typo, or some missing code etc, but...

@interface CalculatorController : NSObject {
    IBOutlet NSTextField *outputField;
    float variable1, variable2;
    float *variable1Pointer, *variable2Pointer;
    float *activeNumberPointer;
}

float variable1 = 0;
float variable2 = 0;
float* variable1Pointer = &variable1;
float* variable2Pointer = &variable2;
float* activeNumberPointer = &variable1;

... you seem to be declaring all the variables above in the interface definition as ivars -- or instance variables that are directly attached to your CalculatorController, and then outside of the interface definition, you are redeclaring these as GLOBAL variables. There's no need for the second set of definitions to use these in your implementation

If you are looking to initialize these instance variables, commonly there is a function called -init that you can override (since you are subclassing NSObject), and its akin to a constructor in other languages. This would live in your implementation file (.m or .mm). It might look something like this:

-(CalculatorController *)init{
   if( self = [super init] ){
       // Do your initialization here etc
   }
   return self;
}

There's also another function -awakeFromNib that you can override if this is being instantiate inside a XIB/NIB file as part of your user interface definition. The -awakeFromNib function gets called when the class and the UI have been loaded.

Edit: to be clear about the pointer business.... self.activeNumberPointer = *self.activeNumberPointer * 10 + buttonClicked;

This is right. You DON'T use the dereference operator on the left hand side, but you DO use the dereference operator on the right hand side, as you are looking for the value of self.activeNumberPointer.

Josh

Josh
A: 
    float variable1, variable2;
    float *variable1Pointer, *variable2Pointer;
    float *activeNumberPointer;
}

float variable1 = 0;
float variable2 = 0;
float* variable1Pointer = &variable1;
float* variable2Pointer = &variable2;
float* activeNumberPointer = &variable1;

You cannot initialize instance variables within the class @interface. What you've actually done here is declared two variables with each of these names.

For each name, one variable is an instance variable, declared within the {…} immediately following the @interface, and the other is a global variable, declared elsewhere in the header. Specifically, you declared the global variables within the @interface, but that doesn't matter: Global variables have no relation to any class or object, and you can put them within or without an @interface with no objection from the compiler.

You initialized the global pointer variables to point to the global float variables, but within the method bodies in the @implementation, instance variables outrank global variables (instance variables are a narrower scope). Moreover, properties are always based on instance variables, not global variables. Therefore, within the methods, you're referring (through the properties) to the instance variables, not the global variables that you initialized.

Instance variables are initialized to nil at instance creation time, so the value of the activeNumberPointer instance variable is NULL. Thus, statements like this one:

*self.activeNumberPointer = *self.activeNumberPointer * 10 + buttonClicked;

read from and write to a null pointer. That's the crash you're seeing.

It's also worth pointing out that this:

*self.activeNumberPointer = …

is not a property assignment, which is to say that it does not send self a setActiveNumberPointer: message; it is a property retrieval (sending a getter message), which returns the pointer. It's worth cutting the property to avoid confusing the two here.

The solution to the crash is to delete the global variables and initialize your instance variables in code. This is what the init method is for. Override init in the typical fashion and set up your initial pointer values there. Access these instance variables directly (without going through a property) throughout the class.

Better yet, dispense with the pointer juggling altogether. Use a BOOL variable to select between them:

BOOL editingVariable2;

Test the Boolean value whenever you need to work with the active variable. In order to not write bothersome if-else statements every time that need comes up, you might encapsulate it into a method, named setValueOfActiveVariable:switchActive:, which does what the first part says and then, if the argument to the second part is YES, switches the active variable.

The Boolean-variable solution will eliminate the risk of accessing an uninitialized or no-longer-valid float pointer from this class entirely.

Peter Hosey