michaelarmstrong / SuperRecord

A small set of utilities to make working with CoreData and Swift a bit easier.
MIT License
366 stars 27 forks source link

[POC] Remove `@objc(className)` #27

Closed ikesyo closed 8 years ago

ikesyo commented 9 years ago

Class names in xcdatamodel (model editor) should be named like ModuleName.ClassName.

Hopefully fixes #21.

ikesyo commented 9 years ago

I think it's the users' choice of which using @objc(name) or naming ModuleName.ClassName in model editor.

PGLongo commented 9 years ago

As said in #21, force the user to make a not standard operation, as you propose, is not a solution of the problem but an alternative of @objc(className).

I think that having multiple solutions could make only confusion and make support more difficult.

The main task should be to help the developer and reduce the workload in an easy, clear and cleaver way.

@michaelarmstrong what do you think?

Thanks anyway

ikesyo commented 9 years ago

In Swift, classes are name-spaced in a module. So naming ModuleName.ClassName in xcdatamodel is not a not standard operation. Because ModuleName.ClassName is a fully qualified class name, it is required operation for Swift nature.

How do you think?

PGLongo commented 9 years ago

I think it not standard because in Xcode 6.2 if you create an Entity and set his name to whatever you want you have to remember to add ModuleName in xcdatamodel

michaelarmstrong commented 9 years ago

I think for iOS 8 and Swift from a style point of view its better to use ModuleName than @objc, however, this Framework does support iOS 7 and ModuleName wasn't working properly when I first created it. Its not enforced what people use (the unit test target is just to run tests, not to edit them). As a user, you can use any you like.

michaelarmstrong commented 9 years ago

I think we should have some proposals here on how to move forward.

PGLongo commented 9 years ago

I think for iOS 8 and Swift from a style point of view its better to use ModuleName than @objc,

Sure, me too but in this way you break the portability with older project without solving the problem at 100, maybe 90%.

Maybe we should wait and try with Swift 1.2 or greater

ikesyo commented 9 years ago

Sorry for the confusion :sweat: , but what I'd like to say was:

Just using NSStringFromClass(self) as is forces the users to use @objc, and they can't use FQCN for their entities. If we use NSStringFromClass(self).componentsSeparatedByString(".").last! as a entity name in the code base, we can support the both options and the users can choice which option to use.

Excuse me for my lack of considering about changes in Xcode 6.2 (is there any changes for Core Data entity handling?) or iOS 7/8 compatibility issues :bow:

ikesyo commented 9 years ago

I pushed reverting changing entity name definition in tests. 0f69564 and bc24950 both pass the test suites. So we can safely support the both options I think.

PGLongo commented 9 years ago

This is a good news, let's think about!

PGLongo commented 9 years ago

@michaelarmstrong, what do you think?

michaelarmstrong commented 8 years ago

Hate to dig this up again, but whats the current state of this?

It seems to satisfy these requirements, if true then it can be merged: