Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decoupling MMRecord and MMRecordResponse #95

Open
andreacremaschi opened this issue Sep 8, 2014 · 3 comments
Open

Decoupling MMRecord and MMRecordResponse #95

andreacremaschi opened this issue Sep 8, 2014 · 3 comments
Milestone

Comments

@andreacremaschi
Copy link
Contributor

Hi Conrad

I am trying to use MMRecordResponse directly to parse and handle the JSON response of a web service bypassing MMRecord and I'd need to use a custom subclass of MMRecordRepresentation, but I noticed that MMRecordResponse depends on a tight coupling between MMRecord and the corresponding NSManagedObject:

- (id)initWithEntity:(NSEntityDescription *)entity {
    if (self = [super init]) {
        NSParameterAssert([NSClassFromString([entity managedObjectClassName]) isSubclassOfClass:[MMRecord class]]);
        _entity = entity;
        _protoRecords = [NSMutableSet set];

        Class MMRecordClass = NSClassFromString([entity managedObjectClassName]);
        Class MMRecordRepresentationClass = [MMRecordClass representationClass];

        _representation = [[MMRecordRepresentationClass alloc] initWithEntity:entity];
        _hasRelationshipPrimaryKey = [_representation hasRelationshipPrimaryKey];
        _prototypeDictionary = [NSMutableDictionary dictionary];
    }
    return self;
}

This is ok when MMRecord is used as designed (subclassing MMRecord) but as a user it would be great to use MMRecordResponse alone. As far as I can understand the rest of the framework, this is the only point that i can see where the marshaler part depends on this design choice: is it correct? What do you think? Would you consider a PR that handle this, defaulting to your implementation but allowing to decouple MMRecordResponse and MMRecord?

More broadly, I think that MMRecord is great when used as designed (model classes as subclasses of MMRecord), but I like the modular nature of the framework and in some occasions as a user I'd rather to handle pure NSManagedObject subclasses, i.e. to decouple local data storage handling and communication with the web services.

@cnstoll
Copy link
Contributor

cnstoll commented Sep 8, 2014

Hola Andrea,

Great thoughts on this topic. I completely agree that the underlying system behind MMRecord should be modular and usable without the standard access through the MMRecord class. The MMRecordResponse should be usable to handle any JSON response. And you should be able to start a request however you want to.

These principles are part of what is guiding MMRecord 2.0.0, which I started a test branch of a few weeks ago. I'm really liking the direction that is taking! I would also love your feedback on that. Particularly, the re-designed MMRecordResponse, and the new MMRecordRequest might be worthy of some attention.

https://github.com/mutualmobile/MMRecord/tree/2.0.0

Thanks,

  • Conrad

@andreacremaschi
Copy link
Contributor Author

Woha! This is the fastest issue resolution ever :)

As I understand it you are completely refactoring MMRecord moving all the state in MMRecordRequest, am I right? I definitely love this approach, as it makes it easier for the developer using the library to separate concerns and to write testable code.

Following the direction you took, I would definitely suggest to further refactor MMRecord and move all the methods meant to be subclassed in a protocol (i.e. + (NSString *)keyPathForResponseObject;, + (Class)representationClass;). This way it should be possible to couple MMRecordRequest and this new protocol, decoupling it from MMRecord. The great advantage would be that it would become possible to generate requests using MMRecordRequest and plain NSManagedObjects, without making all the NSManagedObjects inherit from MMRecord.

Making all your model inherit from MMRecord has been quite a bulky requirement, I must confess that removing it has been in my "I would like but I don't dare to ask" list about MMRecord since some time :)

@cnstoll
Copy link
Contributor

cnstoll commented Sep 8, 2014

Great! Well, I haven't worked on this in a few weeks but I do want to get it out before the end of the year. If you have more comments/suggestions feel free to use this thread to add them.

Thanks,

  • Conrad

On Sep 8, 2014, at 12:12 PM, Andrea Cremaschi [email protected] wrote:

Woha! This is the fastest issue resolution ever :)

As I understand it you are completely refactoring MMRecord moving all the state in MMRecordRequest, am I right? I definitely love this approach, as it makes it easier for the developer using the library to separate concerns and to write testable code.

Following the direction you took, I would definitely suggest to further refactor MMRecord and move all the methods meant to be subclassed in a protocol (i.e. + (NSString *)keyPathForResponseObject;, + (Class)representationClass;). This way it should be possible to couple MMRecordRequest and this new protocol, decoupling it from MMRecord. The great advantage would be that it would become possible to generate requests using MMRecordRequest and plain NSManagedObjects, without making all the NSManagedObjects inherit from MMRecord.

Making all your model inherit from MMRecord has been quite a bulky requirement, I must confess that removing it has been in my "I would like but I don't dare to ask" list about MMRecord since some time :)


Reply to this email directly or view it on GitHub.

@cnstoll cnstoll added this to the 2.0.0 milestone Jan 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants