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

Yams.dump function Any object parameter casted as NodeRepresentable fails #299

Open
ABridoux opened this issue Jan 24, 2021 · 3 comments · May be fixed by #320
Open

Yams.dump function Any object parameter casted as NodeRepresentable fails #299

ABridoux opened this issue Jan 24, 2021 · 3 comments · May be fixed by #320

Comments

@ABridoux
Copy link

Hi! I encountered an error while using the Yams.dump(object:) function because the represent(:) tries to cast an Any value to NodeRepresentable. And it seems to be a Swift problem.

Problem exposition

The error comes from the deserialisation of a Data object to Any. Here is the code that demonstrates that (and the attached playground).

import Foundation

protocol StubProtocol {}
extension Array: StubProtocol {}

let ducks = ["Riri", "Fifi", "Loulou"]
let data = try JSONEncoder().encode(ducks)
let deserializedDucks = try JSONSerialization.jsonObject(with: data)
let castedDeserializedDucks = deserializedDucks as! [String]

print("StubProtocol conformance of 'ducks': \(ducks is StubProtocol)") // true
print("StubProtocol conformance of 'deserializedDucks': \(deserializedDucks is StubProtocol)") // false
print("StubProtocol conformance of 'castedDeserializedDucks': \(castedDeserializedDucks is StubProtocol)") // true

Only when casting the value from Any to [String] the cast to the protocol will work. The same goes for the cast in the represent(:) function:

if let representable = value as? NodeRepresentable

The casting would work only if the Any value is properly casted. But then it will fail with this value children since the call is recursive (e.g. with an array or a dictionary).

Solution

Recursive cast

A solution that the developer can implement is a recursive cast of the Any value to NodeRepresentable. For instance, here is an implementation in Scout.

func castToNodeRepresentable(_ value: Any) -> NodeRepresentable {
    if var value = value as? [String: Any] {
        value.forEach { value[$0.key] = castToNodeRepresentable($0.value) }
        return value
    } else if let value = value as? [Any] {
        return value.map(castToNodeRepresentable)
    } else if let value = value as? Int {
        return value as NodeRepresentable
    } else if let value = value as? Double {
        return value as NodeRepresentable
    } else if let value = value as? Date {
        return value as NodeRepresentable
    } else if let value = value as? Bool {
        return value as NodeRepresentable
    } else {
        return String(describing: value) as NodeRepresentable
    }
}

This does work, especially as the serialisation handles a limited set of types.
But then it seems strange to have to cast an Any value to a NodeRepresentable if the function dump(object:) expects Any.
One solution could be for the library to perform this recursive cast on behalf of the developer. Thus making sure the provided Any value conforms to the NodeRepresentable protocol. But this approach has some weaknesses:

  • performing this operation can take some time on large chunks of data
  • it does not allow customisation of the process. For example the developer could prefer to throw an error rather than fall back on a String.

Requiring a NodeRepresentable

Another solution could be to change the dump(object:) parameter type from Any? to NodeRepresentable

  • This would make the developer aware of the requirement rather than hiding it. Thus making them responsible for conforming Any to this protocol, while also allowing customisation.
  • Requiring a NodeRepresentable prevents a runtime error that can be misleading. After all, if the represent(:) function has to work with a NodeRepresentable object, why wouldn't it ensure it rather than handling an Any value? In fact, I first thought that the problem came from the YAMS library since I was passing it an Any value and there were no requirements on this value. Then I realised the problem came from the deserialisation process of Foundation.

Please let me know if this solution seems valid. If so, I would open a pull request with the proposed modifications.

@ABridoux ABridoux changed the title Yams.dump function Any object parameter casted as NodeRepresentable fails Yams.dump function Any object parameter casted as NodeRepresentable fails Jan 24, 2021
@ABridoux
Copy link
Author

Regarding the conformance of the deserialised Any, I filed a bug on bugs.swift.org.

@jpsim
Copy link
Owner

jpsim commented Mar 2, 2021

Thanks for filing that! I think your proposed solution makes sense as well.

@ABridoux
Copy link
Author

ABridoux commented Mar 3, 2021

Thank you! Someone answered to my issue. That's not a bug in fact.

The thing is, with deserialisation, the Any value is a NSArray and not an Array. Which is why I think it will not conform to NodeRepresentable since the conformance is only declared on the Array type in the library.

A quick fix would be to also add the conformance of NSArray to NodeRepresentable but I would say it's clearer to require a NodeRepresentable value in the function as mentioned.

@ABridoux ABridoux linked a pull request May 15, 2021 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants