convert core to protocol.

[brianmullen]

This allows any object, that conforms to the protocol, the ability to extract values like a dictionary. The only thing that is required is for the object to implement the subscript method: subscript(key: KeyType)

[brianmullen]

It looks like this pull request is triggering an automated build but it fails because no travis.yml file is found. We will need to run the tests again once the pull request #41 has been merged.

[jarsen]

Let's make a decision on this. @bwhiteley what do you think? I feel ok about the proposal.

[jarsen]

I don't know that I'm in love with the name Marshal, but it is a bit of a conundrum since Marshaling is already taken. I think it works.

[bwhiteley]

What about Marshalable?

[jarsen]

I think that would be extra confusing trying to remember which one is Marshaling and which one is Marshalable

[jarsen]

Really, whatever the dictionary is the MarshaledObject. Maybe something more along those lines, since we're already using that for our typealias.

[brianmullen]

I suck at naming and I know that @jarsen already told me that he didnt like Marshalable (that is what I originally named it). So, whatever you guys decide I am sure I will be ok with it :)

[bwhiteley]

This PR will require code changes for projects using the Marshal framework. Is there any way around that?

[jarsen]

I don't know that you suck at naming, naming is just one of the hardest problems in CS ;)

[brianmullen]

@bwhiteley I don't think there is a way around it, if the goal is to become protocol based.

[bwhiteley]

I'm in favor of this change. We just need to name the protocol. I'm OK with MarshaledObject if that's where we are leaning.

[brianmullen]

What would we use as the default type name if we name the protocol that? We still need to have a functioning framework and so we still need a default typealias. The protocol is only going to be useful for others extending our work but I thought we were still going to be a dictionary extraction framework.

[jarsen]

How about: MarshalObjectionable 😆

[bwhiteley]

By "default type name" do you mean this? public typealias MarshaledObject = [String: AnyObject]

What if it was JSONObject? We already also have this: public typealias JSONObject = MarshaledObject

[bwhiteley]

I noticed that UnmarshalUpdating was skipped in the refactor (still uses MarshaledObject instead of Marshal).

[brianmullen]

@bwhiteley yes, I meant MarshaledObject as the default name. I use that to construct a dictionary in my code and so its nice to have a convention to create new objects. I know that JSONObject is available, but I am not using JSON in a lot of my code and so it goes back to my original issue of when the core was not generically named...it doesn't make sense to use a data type with that name in my usage context.

[brianmullen]

@bwhiteley good catch, either I completely overlooked that or it might have been caused from the last pull request that was merged where we changed it to mutating.

[brianmullen]

yeah, it looks like it was reverted when I merged in master

[bwhiteley]

I would argue that the using MarshaledObject as a typealias for [String: AnyObject] is now poorly named. Perhaps MarshalDictionary?

[brianmullen]

@bwhiteley I like that idea :)

[brianmullen]

Would we want it MarshalDictionary or MarshaledDictionary?

[bwhiteley]

That's an interesting question. My first reaction was to say that MarshaledDictionary means a dictionary that has been marshaled (converted to JSON or XML, for example), which is not accurate.

However, in the context of this framework MarshaledDictionary kind of makes sense because the (protocol-conforming) dictionary is actually the portable format in this case. Or at least it is an intermediate step towards a portable format.

[jarsen]

I was thinking about MarshaledDictionary too, actually. Maybe that's a sign.

[jarsen]

But as Bart points out, it could be a bit of misnomer. Maybe it should just be MarshalDictionary. Or maybe something totally different, like UntypedDictionary. Maybe we get rid of the typealias all together and people can just do [String: AnyObject]? And if they want typealias it themselves?

[jarsen]

Especially if we are going more generic with the protocol, then Marshaling and Unmarshaling won't be taking specifically a [String: AnyObject] anyway, right?

[brianmullen]

I think we should provide a default dictionary...its like you were saying about some of the protocols a few days ago...we should provide a pattern to the users or else they will get confused on the usage.

[jarsen]

And, depending on how people are using it, removing the typealias and just naming the protocol MarshaledObject shouldn't break anything?

[jarsen]

What do you mean by "default dictionary"?

[brianmullen]

I currently create instances of MarshaledObject (the default dictionary type for our framework). Changing the name of the protocol to MarshaledObject will fix the API breaking but it will break the places where I create instances (which is quite a bit but I am ok with that name change as long as we have a typealias to replace our current one).

I don't think its obvious to users that a [String: AnyObject] by default will conform to our extraction (unless we assume everyone has read our readme) and so it would be nice to provide a typealias (whatever the name is) to guide users to using the framework without needing to think about what kind of dictionaries may or may not conform to our protocol.

[bwhiteley]

I'm in favor of simply removing public typealias MarshaledObject = [String: AnyObject]

[jarsen]

@psychoticidiot you could still use JSONObject(), or [String: AnyObject](). Or define whatever typealias you'd like locally.

[jarsen]

If we did this, however, we would still have the object -> JSON serialization problem, correct?

[brianmullen]

@jarsen I have issues with JSONObject because I am not using JSON as I mentioned earlier. Yes, I can create my own typealias for [String: AnyObject] but I can do the same thing with our protocols since they provide no default behavior...your argument for having them was that it provides a pattern for users so they aren't confused on how to use it. I am just saying that its the same thing for arguments sake....I am totally willing to create my own typealias but does it cause confusion for our consumers to remove the concept of a pre-defined dictionary?

[jarsen]

I'm not sure I see how removing the typealias removes a pattern for users.

[brianmullen]

As a user, if I implement some of the protocols, its not immediately obvious that I can use [String: AnyObject] since all of the protocols say it takes a MarshaledObject (assuming the new protocol name). This makes it so users will have to analyze our framework to figure what kind of object can/can't be used. By providing a pre-defined typealias, it will be obvious as to what objects are allowed and whats not with users not needing to figure it out....this is exactly what you mentioned about the Marshaling and Unmarshaling protocol as someone mentioned to you they didnt know how to serialize our objects and that is why we provide a protocol.

[jarsen]

If we are going to add this extra protocol, I think we have to make Unmarshaling take that in as it's argument.

Given that, how do you make it immediately obvious that whatever typealias you supply conforms to the new protocol?

I think the patterns will be gleaned from the readme anyway, not from diving into the code. A simple example and a few words would be enough to show that you can marshal a new object from a [String: AnyObject].

[bwhiteley]

After talking to @psychoticidiot about his use case, I change my vote from removing the typealias to using MarshalDictionary.

I think it's harmless, specific, and doesn't have the potential to cause confusion that MarshaledDictionary would.

[brianmullen]

Here is what I would like to see happen: 1) Rename the protocol Marshal to MarshaledObject 2) Rename our typealias MarshaledObject to MarshalDictionary 3) Adjust the Marshaling protocol to what was mentioned above. 4) Add a JSONMarshaling protocol that was mentioned above. 5) Fix all existing protocols to use the new protocol.

[jarsen]

Sounds good to me.

[brianmullen]

Now that we have a typealias for Marshaling, I wonder if we should do the same for Unmarshaling and UnmarshalUpdating. Or do we remove the typealias since we are adding an independent JSON protocol?

[jarsen]

Also, before we merge this, I would like to update the README.

[brianmullen]

I think the only thing pending on this, if we are all in agreement, are the following: 1) Do we keep/remove anyForKey from the protocol 2) Does the protocol JSONMarshaling work for @bwhiteley