Use structs instead of enums for the attribute, relationship, fetched properties and user info keys #221

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

tonyarnold commented Jul 13, 2014

At present, accessing the enum-provided attribute values looks like:

var attributeNameKey = MyClassAttributes.attributeName.toRaw()

This PR replaced the enums with structs to allow:

var attributeNameKey = MyClass.Attributes.attributeName

Personally, I think this is a bit simpler, and the structs being namespaced to the MO subclass is nice.

brentdax commented Sep 2, 2014

I literally got halfway through writing this patch myself when I stopped and came here to discuss the issue and found that it had already been done. +1.

Edit: One unexpected benefit of this change is that, when you're in one class, you can omit the class name to make it more difficult to reference the wrong class's properties:

    return [ Attributes.addedDate, Attributes.name, Attributes.itemDescription, Attributes.removedDate, Attributes.storeURL, Relationships.user ]

brentdax commented Sep 3, 2014

I ended up redoing this for the current version of the project. https://github.com/brentdax/mogenerator/tree/swift-attribute-constants

@kballard kballard added a commit to blackpixel/mogenerator that referenced this pull request Apr 9, 2015

@kballard kballard Merge pull request #221 from tonyarnold/feature/swift-struct-for-attr…
…ibutes-and-relationships

Use structs instead of enums for the attribute, relationship, fetched
properties and user info keys
1c81fbc
Collaborator

justin commented Apr 26, 2015

Still relevant, @tonyarnold?

Contributor

tonyarnold commented Apr 28, 2015

Yeah, I think it is but I'm having a few other conversations around the use of structs over enums that I'd like to see out before making a call on this. rawValue irks me.

I'll update the PR or close it once I've come to a conclusion.

Collaborator

justin commented Mar 22, 2016

We are declaring pull request and issue 0 now that 1.3 is out. If this is still an issue you'd like to see address with 1.30 and going forward, please open a new issue so we can start a fresh discussion. Thank you!

justin closed this Mar 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment