Misc suggestions to `deftable` and `<table-class>`

[PuercoPop]

Oi @eudoxia0, I'm wondering what you think of the following suggestions. If you like any of the following suggestions I'd be happy to implement them.

Make deftable handle slots more like defclass

Currently deftable takes each slot separately and intermingled with options, while defclass takes a list of slots and any number of options aftewards. I'm thinking something along the lines of define-final-class

(defmacro define-final-class (class supers slots &rest options)
  (when (assoc ':metaclass options)
    (error "Defining a final class with a metaclass?"))
  `(defclass ,class ,supers ,slots
         ,@options
         (:metaclass final-class)))

Enforce the abstractp option

We could do something similar to what the previous link does.

(defmethod make-instance ((c abstract-class) &rest junk)
  (declare (ignore junk))
  (if (abstractp c)
      (error "~A is an abstract class." (class-name c))
      (call-next-method)))

Use double helix to hide id slot

This is something I haven't given thought through. The id slot could be thought of as a leaky abstraction. The user didn't define it, so he shouldn't see it. Also is another primary-key is defined it shouldn't be defined. The idiom shown in Avoiding Confusion in Metacircularity: The Meta-Helix could be use to hide it. But on the other hand, the primary key is useful for when comparing the _eq_uality of two objects or when wanting to find out if an instance is stored in the db.

[eudoxia0]

Thanks for taking the time to write these.

Currently deftable takes each slot separately and intermingled with options, while defclass takes a list of slots and any number of options aftewards. I'm thinking something along the lines of define-final-class

In the rewrite branch I've changed the behaviour of deftable to be essentially equivalent to defclass. There are no hidden :accessor or :initarg options and the slots have to be inside a list. The only thing that's done automatically is the id slot and the metaclass option.

We could do something similar to what the previous link does.

Right. This is something I've yet to do in the rewrite branch since I basically never use it. But it would be fairly easy to implement. In fact it could be done in the register-table function, just check if the class is abstract, and if it's not, warn the user and do nothing.

Use double helix to hide id slot

Well, the utility of the id slot is in the fancier methods like save and del. Though in certain things. like association tables where there are only two slots, both of which are unique foreign keys, it's dead weight.

I suppose there could be an id-mixin mixin or something like that where you choose in the deftable macro whether or not to add it. There would have to be some extra checks to prevent re-adding the mixin in subclasses that have the id-mixin way up in the superclass hierarchy. That last sentence could be explained better.

Then methods like save and del could specialize on classes with that mixin and just fail on others.

[PuercoPop]

Glad to learn that the deftable suggestion was already implemented. I guess the question is what is missing for the rewrite branch to be merged?

Btw the new macro signature makes #50 innaplicable, so it may be a good idea to include a snippet to teach Sl{y,ime} to indent deftable properly (apropos)

(define-common-lisp-style "crane"
  (:inherit "modern")
  (:indentation
   (deftable (as defclass))))

In fact it could be done in the register-table function, just check if the class is abstract, and if it's not, warn the user and do nothing.

That is a good idea, although if abstractp means it is not mapped to an SQL table, it may be appropriate to change the name.

id-slot

The problem I was pointing at was that if the user didn't define a slot why should he see when inspecting the class. For example, why if I do:

(deftable user ()
  ((name :type text :uniquep t)
   (age :type integer :nullp nil :initform 18)))

(mapcar #'c2mop:slot-definition-name (c2mop:class-slots (find-class 'user)))

should the user of the table metaclass should see

(ID NAME EMAIL)

that is leaking the abstraction. I was proposing hiding the id slot using the idiom described in the paper. If you agree this is a problem that is.

Well, the utility of the id slot is in the fancier methods like save and del.

I see that save and del assume the presence of a slot named id. It would be better to add a layer of indirection. Something along the lines of (primary-key instance), returning the value of the primary key; and (primary-key-name instance) returning the slot-name. Another alternative woud be for (primary-key <instance>) to return the primary key slot. I am aware that this would disallow multi-column primary key. However this can chalked up to a limitation due being out of the scope of the use case of the design. (Later, if we go with the latter API, primary-key could return a list of slots or a slot for multi column primary keys).

[eudoxia0]

Glad to learn that the deftable suggestion was already implemented. I guess the question is what is missing for the rewrite branch to be merged?

Migrations support. There's already code to diff tables. It's all modular and CLOS-y and neat.

What's missing is,

1. Storing the migrations (I am not sure what the best way of doing this is)
2. Keeping track of the current migration
3. Actually applying the migrations

I don't want to merge it even though it's infinitely better with the automatic migrations crap because "muh migrations" is a central selling point of ORMs.

That is a good idea, although if abstractp means it is not mapped to an SQL table, it may be appropriate to change the name.

Hm, you are right. But I don't know what else to call it. unmapped is perhaps too long.

You know, actually, it's not necessary. All you have to do is not register the table.

that is leaking the abstraction. I was proposing hiding the id slot using the idiom described in the paper. If you agree this is a problem that is.

I've yet to read it.

It would be better to add a layer of indirection. Something along the lines of (primary-key instance), returning the value of the primary key; and (primary-key-name instance) returning the slot-name. Another alternative woud be for (primary-key ) to return the primary key slot. I am aware that this would disallow multi-column primary key. However this can chalked up to a limitation due being out of the scope of the use case of the design. (Later, if we go with the latter API, primary-key could return a list of slots or a slot for multi column primary keys).

I actually like this idea. The method doesn't even have to be implemented explicitly, you simply have to filter all the slots where :primaryp is t.

[PuercoPop]

Hm, you are right. But I don't know what else to call it. unmapped is perhaps too long.

What about mixin? it is that use case of not mapping to a table the class IIUC.

I've yet to read it.

Although I recommend you to read it when you have time, it would be presumptous of me to expect you. This is (half of ) the gist of it, but enough to get the idea. The idea is that one creates a copy of the user-defined class with extra slots, and hook into slot-value-using-class so that accessing slots of the user-defined class defers to the enhanced class. Also there is a map between each instance of the user-defined class to an instance of the enhanced one (implemented-by). While writing what I expect to build a simple PoC, I've realized that it is probably more lines of code that its worth for the problem of slots not defined by the user being created on its class. (Although it would make a good blog post if finished :smile: )