justinmeiners / ios-color-wheel

A fully scalable, dynamically rendered color wheel for iOS.
82 stars 15 forks source link

Modernize code; ARC enable #1

Closed cbess closed 9 years ago

cbess commented 11 years ago

Code cleanup (simplify) and modernizing.

justinmeiners commented 11 years ago

I can't move to ARC I still need to support traditional memory management.

I will take some of your simplification into account, but I don't see the purpose of accessing instance variables internally through a property (getter). I also don't see the advantage of CGRectGetWidth compared to bounds.size.width, the code is often longer, and I would argue less readable.

Thanks for the request,

I will integrate some of the changes by looking at your code.

justinmeiners commented 11 years ago

I have made updates based on your suggestions.

Thank you!.

cbess commented 11 years ago

I'm only responding in detail to help, not to argue. I have a little experience under my belt :)

I don't see the purpose of accessing instance variables internally through a property (getter).

It is good programming practice to use getters everywhere else except during object construction (in ObjC). Getters are used not only to wrap internal vars, but as a way to further future proof the code. Here are a few other reasons.

I also don't see the advantage of CGRectGetWidth compared to bounds.size.width, the code is often longer, and I would argue less readable.

Here are reasons to use CGGeometry functions.

I hope that helps. If you have any other questions about it, please let me know.

justinmeiners commented 11 years ago

Sorry I don't want to argue either :( but I think I have good reasons for not doing it.

It is good practice internally for setters (outside constructor), for obvious property functionality reasons, I don't think its also a good practice for getters. Its an extra message send, and an extra thing the type. That stack overflow question is about general setters and getters for public interface which I support, none of those benefits really apply to internally using a getter. The main argument for public interface is you can change how it works and it doesn't affect other classes using it that you don't have control over, I guess you could argue it might protect a class from internal changes, but if you change how the getter works, you can also change internally how the class works in minimal time compare to outside code.

Interesting I did not know that - the Rect thing is for a specific desired normalizing functionality not code style (Documentation, question, and mailing list) "Your application can standardize a rectangle—that is, ensure that the height and width are stored as positive values—by calling the CGRectStandardize function. All functions described in this reference that take CGRect data structures as inputs implicitly standardize those rectangles before calculating their results. For this reason, your applications should avoid directly reading and writing the data stored in the CGRect data structure. Instead, use the functions described here to manipulate rectangles and to retrieve their characteristics."

I don't think coupling is an issue between Rect -> Size -> Width. It is how they are fundamentally defined. I don't think this is a good example of coupling http://en.wikipedia.org/wiki/Coupling_(computer_programming)

Given the normalizing safety functionality your probably right, its probably safest to use CGRectGetWidth.