Objectmapper: Cannot get mapping to work without MappableCluster

[krummler]

Since MappableCluster was removed, I cannot get dynamic mapping to work.

I'm specifically talking about this change:
https://github.com/Hearst-DD/ObjectMapper/commit/b40c2493cfd9940a21059104be63ab07b5d87a9d

Before I had an extension to the base class of my rest model, which implemented MappableCluster and objectForMapping. Now that it's not in a separate protocol anymore, the extension function never seems to get called.

The function does get called when I move it directly in the rest entity class, but my datamodel is generated (to ensure it complies to the backend specs). Besides, it's weird to be forced to put this code directly into the class.

[tristanhimmelman]

Hey there,

In my test case in the ObjectMapper project this feature is working as expected. The objectForMapping function is just in the super class and it is still called. See the example below

class Vehicle: Mappable {

    var type: String?

    class func objectForMapping(map: Map) -> Mappable? {
        if let type: String = map["type"].value() {
            switch type {
                case "car":
                    return Car(map)
                case "bus":
                    return Bus(map)
                default:
                    return nil
            }
        }
        return nil
    }

    required init?(_ map: Map){

    }

    func mapping(map: Map) {
        type <- map["type"]
    }
}

class Car: Vehicle {

    var name: String?

    override func mapping(map: Map) {
        super.mapping(map)

        name <- map["name"]
    }
}

class Bus: Vehicle {

    override func mapping(map: Map) {
        super.mapping(map)
    }
}

let carName = "Honda"
let JSON = [["name": carName, "type": "car"], ["type": "bus"], ["type": "vehicle"]]

if let vehicles = Mapper<Vehicle>().mapArray(JSON){
    XCTAssertNotNil(vehicles)
    XCTAssertTrue(vehicles.count == 3)
    XCTAssertNotNil(vehicles[0] as? Car)
    XCTAssertNotNil(vehicles[1] as? Bus)
    XCTAssertNotNil(vehicles[2])
    XCTAssertEqual((vehicles[0] as? Car)?.name, carName)
}

[krummler]

In the example you subclass the entity, I use an extension on a base class to add the function like so:

extension Linkable: MappableCluster {

    public static func objectForMapping(map: Map) -> Mappable? {
        // implementation here
    }

}

This doesn't work anymore, the extension doesn't add any protocol conformance (since Linkable itself already conforms to Mappable). Adding this method won't actually let it be used.

[tristanhimmelman]

Right. You will need to include the objectForMapping function directly in the entity or where ever you implement Mappable.

[tdekker]

The new structure is much weaker. You should be able to implement the custom mapping in an extension, especially if your entities are generated. It was a change without any solid arguments and should be reverted if you ask me.

[tristanhimmelman]

The change was made to simplify the code base. The objectForMapping function allows more than just dynamic mappings, so I was not happy with the naming of its Protocol. Instead of renaming it, I integrated it as a core part of Mappable. I will keep your feedback in mind and see if I get any other feedback about the change.

Anyhow, you can still achieve exactly what you were describing in your first post with the code below. This allows your custom mapping (objectForMapping) to be in an extension outside of your generated entity.

class Base: Mappable {
    // generated entity
}

extension BaseClass {
    static func objectForMapping(map: Map) -> Mappable? {

    }
}

[krummler]

I can understand you'd want to keep the codebase simple. In this case would say the purpose of objectForMapping is different from the other functions described in the protocol, therefor I'm not sure they really belong together?

Ideally I would say objectForMapping belongs in neither Mappable nor MappableCluster, as it's more of a config thing, rather than being part of the entity (or it's specific mapping) itself.

The solution you described doesn't work unfortunately, the extension doesn't provide any additional protocol conformance, therefor the function doesn't get called.

Simply said:

Doesn't work (function doesn't get called)

extension BaseClass {
    static func objectForMapping(map: Map) -> Mappable? {

    }
}

Works:

extension BaseClass: MappableCluster {
    static func objectForMapping(map: Map) -> Mappable? {

    }
}

[tristanhimmelman]

You're right, that solution does not work... I thought I had tested it successfully but I guess not...

I would argue the objectForMapping still relates to specific Entities. For instance, if you are using it to return a cached object from a DB, you need to know what type of Entity you are returning. If we moved the function outside of the Entity, you would only know this if the type is specified directly in the JSON.

Another situation is the dynamic mapping. I think the most common use case of dynamic mapping would be subtypes of a base class. In this situation, the objectForMapping function only needs to know how to distinguish between its subtypes. If we move it outside the entity, it needs to be able to distinguish between all possible mappings, which could potentially case conflicts depending on the naming conventions.

I will put some more thought into this issue. Perhaps moving the function back into it's own protocol is a more flexible solution. However, I would need to find a better name than MappableCluster as that does not really capture its functionality properly.

[krummler]

Hello Tristan,

I was wondering if you know what direction you would like to go yet? I'm still stuck on version 1.2.0 and I don't like falling behind ;).

[tristanhimmelman]

Hi all, I have just released a new version of ObjectMapper which separates objectForMapping into a new protocol named StaticMappable.

Cheers

[krummler]

The tag 1.4.0 exists, but the ObjectMapper.podspec still points to 1.3.0 :(.

Edit: I fixed it for now by manually pointing to the repo, the changes do fix the problem and I was able to upgrade without any problems. Thanks a lot of the change!

[tristanhimmelman]

Sorry, I forgot to push the podspec. It's now up