Split the protected interface of SRRecorderControl into a separate header?

[zoul]

The public interface of the SRRecorderControl class is quite big. And a good half of the methods are not intented for public consumption – some of them could be used by subclasses, some should be private.

Can we clean the public interface to really keep just the public methods and properties? I suggest moving the private stuff (if any) into an extension in the implementation file and the protected stuff into a separate category header, something like SRRecorderControl+Subclass.h (there are some standard Cocoa classes split this way).

I think the change would make the public API more readable. What do you think? I am happy to send a PR.

[Kentzo]

In fact I thought of splitting it into categories multiple times. I agree it's a bit huge and overwhelming.

I'm against private methods in the implementation file (with very few exceptions). Every method in current public interface is designed to be handy and useful in subclassing, learning and debugging. Imagine you would need to switch between h/m files all the time you're learning new code. I find it awful.

I agree, we could split interface into categories but we should keep them in the same header.

SRRecorderControl (Layout)
SRRecroderControl (Accessibility)
…

Could you copy list of methods declared in the interface so we can work out them?

[zoul]

I don’t mind switching headers that much. My primary objective is keeping the main header easily manageable, digestable. If we split the properties and methods to several categories, that’s a step forward, but by keeping the categories in the same header it’s still a lot to look at. Especially given that most people just care about the public methods.

We could at least split the public interface and the protected one, meant for subclassing. That’s a good start, even if we keep them in the same header. Here’s the list of properties and methods, can you split those into the public / protected categories?

@property (assign) IBOutlet NSObject<SRRecorderControlDelegate> *delegate;
@property (readonly) NSUInteger allowedModifierFlags;
@property (readonly) NSUInteger requiredModifierFlags;
@property (readonly) BOOL allowsEmptyModifierFlags;
@property BOOL drawsASCIIEquivalentOfShortcut;
@property BOOL allowsEscapeToCancelRecording;
@property BOOL allowsDeleteToClearShortcutAndEndRecording;
@property (nonatomic, getter=isEnabled) BOOL enabled;
@property (nonatomic, readonly) BOOL isRecording;
@property (nonatomic, copy) SRShortcut *objectValue;
@property (nonatomic, copy) NSDictionary *dictionaryValue;
- (void)setAllowedModifierFlags:(NSUInteger)newAllowedModifierFlags requiredModifierFlags:(NSUInteger)newRequiredModifierFlags allowsEmptyModifierFlags:(BOOL)newAllowsEmptyModifierFlags;
- (BOOL)beginRecording;
- (void)endRecording;
- (void)clearAndEndRecording;
- (void)endRecordingWithObjectValue:(SRShortcut *)anObjectValue;
- (NSBezierPath *)controlShape;
- (NSRect)rectForLabel:(NSString *)aLabel withAttributes:(NSDictionary *)anAttributes;
- (NSRect)snapBackButtonRect;
- (NSRect)clearButtonRect;
- (NSString *)label;
- (NSString *)accessibilityLabel;
- (NSString *)stringValue;
- (NSString *)accessibilityStringValue;
- (NSDictionary *)labelAttributes;
- (NSDictionary *)normalLabelAttributes;
- (NSDictionary *)recordingLabelAttributes;
- (NSDictionary *)disabledLabelAttributes;
- (void)drawBackground:(NSRect)aDirtyRect;
- (void)drawInterior:(NSRect)aDirtyRect;
- (void)drawLabel:(NSRect)aDirtyRect;
- (void)drawSnapBackButton:(NSRect)aDirtyRect;
- (void)drawClearButton:(NSRect)aDirtyRect;
- (BOOL)isMainButtonHighlighted;
- (BOOL)isSnapBackButtonHighlighted;
- (BOOL)isClearButtonHighlighted;
- (BOOL)areModifierFlagsValid:(NSUInteger)aModifierFlags forKeyCode:(unsigned short)aKeyCode;
- (void)propagateValue:(id)aValue forBinding:(NSString *)aBinding;

[zoul]

I’ll have a shot at the first iteration. The properties are obviously public:

@property (assign) IBOutlet NSObject<SRRecorderControlDelegate> *delegate;
@property (readonly) NSUInteger allowedModifierFlags;
@property (readonly) NSUInteger requiredModifierFlags;
@property (readonly) BOOL allowsEmptyModifierFlags;
@property BOOL drawsASCIIEquivalentOfShortcut;
@property BOOL allowsEscapeToCancelRecording;
@property BOOL allowsDeleteToClearShortcutAndEndRecording;
@property (nonatomic, getter=isEnabled) BOOL enabled;
@property (nonatomic, readonly) BOOL isRecording;
@property (nonatomic, copy) SRShortcut *objectValue;
@property (nonatomic, copy) NSDictionary *dictionaryValue;

Now the obviously public methods:

- (void)setAllowedModifierFlags:(NSUInteger)newAllowedModifierFlags requiredModifierFlags:(NSUInteger)newRequiredModifierFlags allowsEmptyModifierFlags:(BOOL)newAllowsEmptyModifierFlags;
- (BOOL)beginRecording;
- (void)endRecording;
- (void)clearAndEndRecording;

How about the labels and attributes? What’s the expected use case for these?

- (NSString *)label;
- (NSString *)accessibilityLabel;
- (NSString *)stringValue;
- (NSString *)accessibilityStringValue;
- (NSDictionary *)labelAttributes;
- (NSDictionary *)normalLabelAttributes;
- (NSDictionary *)recordingLabelAttributes;
- (NSDictionary *)disabledLabelAttributes;

These are methods I would make private or protected:

- (void)endRecordingWithObjectValue:(SRShortcut *)anObjectValue;
- (NSBezierPath *)controlShape;
- (NSRect)rectForLabel:(NSString *)aLabel withAttributes:(NSDictionary *)anAttributes;
- (NSRect)snapBackButtonRect;
- (NSRect)clearButtonRect;
- (void)drawBackground:(NSRect)aDirtyRect;
- (void)drawInterior:(NSRect)aDirtyRect;
- (void)drawLabel:(NSRect)aDirtyRect;
- (void)drawSnapBackButton:(NSRect)aDirtyRect;
- (void)drawClearButton:(NSRect)aDirtyRect;
- (BOOL)isMainButtonHighlighted;
- (BOOL)isSnapBackButtonHighlighted;
- (BOOL)isClearButtonHighlighted;
- (BOOL)areModifierFlagsValid:(NSUInteger)aModifierFlags forKeyCode:(unsigned short)aKeyCode;
- (void)propagateValue:(id)aValue forBinding:(NSString *)aBinding;

[JanX2]

What happened to this issue? Is the limiting factor time? I could do this.

[Kentzo]

@JanX2 Ah I'm not sure that's even needed.

My rationale was to have a subclassable UI control and I wanted all methods to be public. For that reason splitting the code into categories is as good as #pragma mark.

In other hand I realize that interface complexity increases the learning curve. But that should be better addressed with documentation and examples.

If you have a different opinion, feel free to share.

[JanX2]

It would be nice to have it be immediately obvious, what is necessary and what is optional. I read through the heads and it was too much information to grasp all at once. Better to have something that can be consumed incrementally.

[Kentzo]

I have tried to split headers into sections by Task as Apple's docs do.