drenganathan / Converter

Celsius <=> Fahrenheit Temperature Converter
0 stars 0 forks source link

Please review the first assignment #1

Open drenganathan opened 11 years ago

drenganathan commented 11 years ago

Hi Tim,

My app is a complete, Can you please review it and give your comments?

I am not sure about what is the best way to keep the track of the recent updated textField among the list of textFields. Do you have any pointers, please?

Thanks, Renga

/cc @nesquena @timothy1ee

timothy1ee commented 11 years ago

Looks good, except you shouldn't declare the UITextField *userInputField in the implementation as you have done. Instead, declare it as a private property in the class extension. i.e.,

@interface ConverterViewController ()

- (float)fahrenheit2Celsius:(float)fahreneit;
- (float)celsius2Fahrenheit:(float)celsius;

@property (nonatomic, weak) UITextField *userInputField;

@end

Also, it's possible that I hit the convert button before I end editing in the UITextField, so it may be that using - (BOOL)textFieldShouldEndEditing:(UITextField *)textField doesn't give you what you want. Maybe you should save the userInputField when editing begins, not ends.

Finally, for the convert button, you bound to the "touch down" event. It's more common to use the "touch up inside" event. In other words, the action should happen when the user lifts their finger up from the button. If you think about the apps that you use, that's what other people do.

drenganathan commented 11 years ago

Thanks Tim for the valuable feedback. I have incorporated the change as you have suggested.

Thanks, Renga

/cc @nesquena @timothy1ee