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

Feature request: Store user data in cells #4

Closed
mattpolzin opened this issue Feb 1, 2020 · 6 comments
Closed

Feature request: Store user data in cells #4

mattpolzin opened this issue Feb 1, 2020 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@mattpolzin
Copy link

It would be nice to store some user info along with each cell so that I don't need to store a hash of additional cell info outside of HexGrid since I am already using HexGrid to find the cell I want based on its pixel or cube coordinates.

Something as simple as the ubiquitous public var userInfo: [AnyHashable: Any] would do the trick.

@mattpolzin
Copy link
Author

Hmmm. I see now that this is not so simple. You cannot simply add that property and still retain Codable autosynthesis. I guess the three options that immediately come to my mind are (1) AnyCodable, but that either comes at an implementation cost or else an added dependency, (2) Make Cell an open class so properties can be added to a subclass in other modules, (3)use a custom implementation of Codable that ignores the userInfo.

I see pros and cons to all three, but I think my least favorite is (3) because it is not very common to ignore stored properties when encoding/decoding so documenting the behavior might not be enough to avoid surprise.

@fananek
Copy link
Owner

fananek commented Feb 2, 2020

I’m currently quite busy with other stuff, but let me think about it. Hopefully I’ll be able to look at this one during next week.

@mattpolzin
Copy link
Author

No rush! Just playing with your library in a pet project and posting all the ideas I come up with along the way.

@fananek fananek added the enhancement New feature or request label Feb 3, 2020
@fananek fananek self-assigned this Feb 3, 2020
@fananek
Copy link
Owner

fananek commented Feb 4, 2020

Hi Matt, you are right. Decoding Any type is not that simple task. Here are some of my thoughts.
(1) AnyCodable is probably the most straightforward but as you mentioned it would mean additional dependency which I would like to avoid. HexGrid currently use only Foundation. Another thing is that with large data sets it could have negative impact on performance.

(2) Setting access level of Cell class to open seems to be an option. However, it probably decrease usability of the library. On the other hand it gives you a freedom to do whatever you need.

(3) Frankly speaking, I don't like the idea of ignoring userInfo property. First, we loose the beauty and simplicity of Codable. Second, in most use cases people would like to store their grid including userInfo and/or decode it later.

(4) We can implement own custom encoding/decoding supporting Any type. This will be very similar approach like AnyCodable. Probably not the best idea. It doesn't make sense to reinvent the wheel.

(5) Do we need to work with Any? Isn't it an overhead for most use cases? Would it be sufficient to expect/support specific data type(s) covering majority of cases? Just thinking aloud.

(6) There is an other smart way to use generics in conjuction with Codable which I'm not aware of.

Conclusion: Have to think about it a bit more. At this point, option 2 (eventually 5) seems to be a right way from my perspective.

@mattpolzin
Copy link
Author

That’s a nice write up on the subject. Since you are clearly considering a broad array of possible paths, I’ll add two options. Really, I’ll flesh out option 6 and then add 7.

(6) Make Cell generic. This either requires you to make any type that uses Cell also generic or else introduce a type erasure pattern (AnyCell) or a protocol that describes all of Cell’s requirements.

class Cell<UserInfo: Codable> {
  ...
  var userInfo: UserInfo
  ...
}

(7) Go the protocol oriented approach. This is actually partially overlapping with (6) but generally I would design Cell as a value-type with this approach so I’m listing it separately. First, create a Cell protocol that has no associated type or self-constraints to use as an existential anywhere you currently use the Cell class type. Second, either create a default cell type for users who don’t need anything special (maybe a BasicCell) and your users are welcome to create their own types that conform to the Cell protocol and have whatever behavior they want. As I was mentioning, the concrete types conforming to the Cell protocol would be value-types (structs and enums) with this design.

Anyway, I’d say you and I generally agree on which approaches are favorable. My personal preferences, not to be construed as judgmental of your final decision: (7), (6), and (2).

@fananek
Copy link
Owner

fananek commented Mar 2, 2020

Available in release 0.3.2. I've decided to implement custom type "Attribute" using erasure pattern. At this moment it seems to be a good balance between library usability, flexibility and implementation complexity.

I can imagine more "Swifty" approach but considering all obstacles related to Codable vs. generic heterogeneous collections, it doesn't seem to be possible at this moment. I've spent a lot of time studying Swift documentation as well as going thru others solutions. Maybe it will be possible with some future version of Swift. In such case I will gladly open this topic again and eventually refactor the code.

@fananek fananek closed this as completed Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants