fananek / hex-grid

HexGrid library provides easy and intuitive way of working with hexagonal grids.
MIT License
45 stars 8 forks source link

Feature request: Store user data in cells #4

Closed mattpolzin closed 4 years ago

mattpolzin commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

fananek commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.