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

add IEntityMap marker protocol #4

Closed
wants to merge 2 commits into from

Conversation

swannodette
Copy link

It's useful to be able to differentiate entity maps from plain
maps. This patch uses specify! to mark the returned maps as entity
maps.

David Nolen and others added 2 commits April 26, 2014 18:53
It's useful to be able to differentiate entity maps from plain
maps. This patch uses `specify!` to mark the returned maps as entity
maps.
:cardinality -> :db/cardinality and :many ->
:db.cardinality/many. Support :db/valueType :db.type/ref in the
schema. Change entity so that refs are followed. Update README and
tests.
@swannodette swannodette mentioned this pull request Apr 27, 2014
@tonsky
Copy link
Owner

tonsky commented Apr 27, 2014

I always was curious—what’s the grand idea behind marker protocols? I understand ordinary protocols, e.g. when something is ISequable, you can call -seq on it. But when something is ISequential, or IEntityMap—what’s the actual meaning of this? I guess, it means somebody decided to mark something that way, for the reasons unknown. But what should I do with that information? Should I mark my data structure ISequential? What would break if I did not? How is map marked as IEntityMap different from ordinary map? Why should it be treated differently? If I build entity myself, without doing entity call, should I do satisfy IEntityMap? Maybe I don’t see something, but for me it’s just useless to provide protocol without methods. Can you elaborate on that?

@swannodette
Copy link
Author

@tonsky Marker protocols are extremely useful - we now know that the map came from a DataScript database. For example you could imagine in Om being able to call transact! on an IEntityMap instance and because we know it's an IEntityMap and not just a plain map we can trigger a DataScript DB transaction.

@tonsky
Copy link
Owner

tonsky commented Apr 27, 2014

Sorry, I don’t get it. Ok I have IEntityMap. What if I assoc? What if I dissoc? Why I cannot transact with map that come not from DB? For me, entity is just a map. You can put any map to database. It doesn’t matter where it comes from. We shouldn’t treat some maps different from others and build any expectations on that. Especially when it’s as invisible as marker protocols are.

@swannodette
Copy link
Author

@tonsky entities from the database simply need to be marked (think about defrecord and the record? predicate), I don't think my idea needs any more justification.

If you don't want to add things like this I will go ahead with starting a separate client side Datomic-like effort with the properties that I believe are needed.

@robknight
Copy link

@tonsky I think the idea here isn't about changing how DataScript treats records that are received in transactions, but about enabling third-party libraries to change their behaviour when interacting with data queried from the DB.

Om manages application state by wrapping values in 'cursors'. Right now there are cursor implementations for standard maps and vectors, and calling om/transact! on these just updates the values stored in an atom. I'm guessing that what @swannodette wants to do here is create an Om cursor implementation for DataScript entities.

Without the marker protocol, Om will just treat entities from DataScript as plain maps. They'll get wrapped in a standard map cursor, and any time om/transact! is called on them, a new value will be stored in the state atom. However, if Om can detect (via the marker protocol) that it's dealing with a DataScript entity, it can wrap the entity in a different cursor implementation. This cursor implementation would trigger a DB transaction whenever om/transact! is called on it, enabling Om to use DataScript as a back-end for application state instead of plain ClojureScript maps and atoms.

@tonsky
Copy link
Owner

tonsky commented Apr 28, 2014

@robknight Data queried from DB is just data. It may come from q call of from entity call or from transaction report. These are all valid examples of data, and these are all perfectly legal ways to get it from DB. I don’t understand why data that comes from entity should be treated special.

What you say about Om cursors can be very easily implemented without such a strange thing as “marker protocols”, in a very straightforward way: by providing an implementation for DataScript entity cursor:

(defrecord DataScriptEntityCursor [conn eid ...]
  IOmCursor
  (transact! [_ k v]
    (d/transact! conn [(assoc (d/entity @conn eid) k v)]))
  (deref [_]
    (d/entity @conn eid))
  ...)

I know how Om implements cursors (I wrote official wiki doc about them), but for me it always seemed like a lot of unnecessary magic. It’s too fragile, hard to understand and has questionable user experience (now it’s cursor, now it’s not). Reliance of Om for third-party libraries to implement marker protocols is just a sign that something is wrong about that design.

It’s easy to fix them, though: just make cursors explicit, like atoms or refs are explicit. Atoms do not pretend to work like a map or a vector. They are not ILookup nor ISequential. They are containers with two operations: get value from it and put value inside. If cursors worked that way, it’d be much easier to understand and work with them. @swannodette what do you think?

@swannodette
Copy link
Author

@tonsky marker protocols are not strange ISequential has no methods and is incredibly useful. Being able to detect entity maps is going to be useful in many cases particularly for library designers. Like I said I don't think this needs any justification beyond what I've already stated.

Cursors are a useful underlying concrete implementation of an abstraction, but they were designed such that if some better underlying concrete implementation came along users could migrate to that implementation with zero headache. Exposed cursors prevents flexibility to move to some other more appropriate underlying implementation - whether that's entity maps, bound queries, etc. etc. So while it may seem like "magic" this is actually an important tradeoff I'm not going to give up on yet. It's not by coincidence that internally Om relies very little on the implementation details of cursors.

You've done a great job blazing a trail with DataScript, however I have some ideas of my own how the code should actually be implemented and I've ended up starting on my own effort. Thanks much!

@tonsky
Copy link
Owner

tonsky commented Apr 29, 2014

I won’t merge IEntityMap.
Follow refs is a subject to recursive infinite loops because it’s not lazy.
Thanks anyway!

@tonsky tonsky closed this Apr 29, 2014
@tonsky
Copy link
Owner

tonsky commented Aug 25, 2014

DataScript 0.3.0 now has lazy entities type & supports :db.type/ref auto-wrapping into entities

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 this pull request may close these issues.

3 participants