pharo-project / pharo

Pharo is a dynamic reflective pure object-oriented language supporting live programming inspired by Smalltalk.
http://pharo.org
Other
1.21k stars 355 forks source link

Slot template definition can be confusing to read (instance vs. class side methods, keyword chaining vs. unary chaining, ...) #16741

Open astares opened 4 months ago

astares commented 4 months ago

At the moment and by design Pharo accepts/uses often the slot name as pairs of slotname (a symbol) on the left and the slot class on the right with the binary message #=> inbetween.

Object << #Bar
    slots: { #slotVarName1 => SlotClass1.
               #slotVarName2 => SlotClass2
               ...
     };
    package: 'Foo'

one can also use the notion of

Object << #Bar
    slots: { #slotVarName1 => SlotClass1 method1: arg1.
               #slotVarName2 => SlotClass2
               ...
     };
    package: 'Foo'

where #method1: always is an instance side keyword method and unfortunately can not be an unary message as I will explain.

It's working - yes - but this limits and the way we display the defined slots in this design is unfortunately not optimal and very often also very CONFUSING about which methods are used and if they are from instance or class side.

Let me explain in more detail:

  1. To define a slot the binary message #=> is used. The slot is already instantiated during that process of calling #=>. So it is enough to write the slot class name as an argument:
#var => BooleanSlot

Yes - this safes a few bytes/characters to type as one does not have to write "BooleanSlot new" on the right side. But it comes with cost(s).

Instantiation typically happens in Variable class>>#named:

named: aSymbol
    self checkValidName: aSymbol.
    ^ self new
        name: aSymbol;
        yourself

As the #=> looks like an arrow it can be interpreted as "points to" - so it looks like the #var slot is represented by/points to the class BooleanSlot (instead of the already created instance). In fact the full expression gives an own instance of BooleanSlot (stored in the slot behind the scenes) which is not very obvious from what is written there

image

In a full class definition this looks like:

Object << #Bar
    slots: { #var => BooleanSlot };
    package: 'Foo'
  1. When one is using a slot definition with instance side slot method - especially keyword messages then in the definition these selectors directly follow the class name. So for instance:
Object << #Bar
    slots: { #var => BooleanSlot };
    package: 'Foo'

On a first short look when system is displaying it this gives the impression that #inverse:inClass: is a class side method of ToManyRelationSlot. As the binary message #=> has a higher priority than the keyword message the left part evaluates first (and providing the instance). Then the instance method #inverse:inClass: is sent to the slot instance which is not really obvious from the declaration:

Object << #Bar
    slots: { #people => ToManyRelationSlot inverse: #x inClass: #SlotExamplePerson };
    package: 'Foo'

So this is confusing again as on a first quick look it looks more like the #people slot is pointing to a ToManyRelationSlot instance created with a class side message sent to ToManyRelationSlot on the right side.

In fact evaluating the right side will give a DNU as #inverse:inClass: is not on class side but instance side:

ToManyRelationSlot inverse: #x inClass: #SlotExamplePerson

and yes even when one reflects correctly that the left side with the binary #=> message is evaluated first it is not onbious that an instance is created.

image

  1. Now there is also the possibility to define the slot with cascaded keyword messages. The image includes "ExampleSlotWithFluidAPI" as an example and testExampleSlotWithFluidAPI shows how to use this slot in a definition:
Object << #Bar
    slots: { #slot => ExampleSlotWithFluidAPI value1: #testvalue1; value2: #testvalue2 };
    package: 'Foo'

Also in such cascaded keyword messages it looks as if the "cascaded" messages are sent to the class ExampleSlotWithFluidAPI and not an instance of it (which is created through the #=> message). The class side does not understand #value1: or #value2:

  1. While the ExampleSlotWithFluidAPI from example 3 works for cascaded messsages when they represent keyword - but it will not work for cascaded or non-cascaded unary messages. As unary messages will have a higher precedence vs. the binary message they would be evaluates first:
Object << #Bar
    slots: { #slot => SomeFieldDefinitionSlot beEnabled; withTemplate: 'Prefilled' };
    package: 'Foo'

and evaluating the fulll slot expression is then not gonna work with having #beEnabled only on the instance side as it would be evaluated first (if you run right side which would be wrong but also when you run the full slot definition expression only)

So in

#slot => SomeFieldDefinitionSlot beEnabled; withTemplate: 'Prefilled'

the #beEnabled would be sent first (instead of the #=>) and needs to be on class side then.

So any parameterization of a slot using an unary method/unary method approach would not work here. Confusing again as it would not work for such unary messages, while it only works for keyword messages.

astares commented 4 months ago

All not ideal - I thought long on how this could be improved and that it works better for all kind of messages. If we would like to stay with that notion at the moment I can only think of two possible solutions/improvements:

OPTION A - additional parantheses

In the printing the binary message #=> could be respresented with additional parantheses:

Object << #Bar
    slots: { (#slot => SomeFieldDefinitionSlot) beEnabled; withTemplate: 'Prefilled' };
    package: 'Foo'

as this has a higher order than any follow up unary, binary or keyword message independent if chained or cascaded. This way it would also be more obvious that it gets evaluated first but still not obvious enough.

If there is no message the parantheses must not be there:

Object << #Bar
    slots: { #slot => BooleanSlot };
    package: 'Foo'

OPTION B - additional #new or class side messages

In this option the right side side of #=> is expected to be able to also already create the instance. If only a class is given it can work as before. So if a class is given as an argument to #=>the slot will be created while if an instance is given it migh be initialized with the slot name (if not already set).

For instance in the printing using #printOn: the right part could alway presented with an additional " new". As an unary message is evaluated first anyway this would make the definition more clear and one can deal with any follow up unary, binary or keyword message independent if chained or cascaded.

Object << #Bar
    slots: { #slot => BooleanSlot new };
    package: 'Foo'

would make it more obvious.

Also all other variants would make it more clear and would work

Object << #Bar
    slots: { #slot => SomeFieldDefinitionSlot new beEnabled; withTemplate: 'Prefilled' };
    package: 'Foo'

one would be able to use also class side slot methods for creation and parameterization of the slot instance:

Object << #Bar
    slots: { #slot => SomeFieldDefinitionSlot defaultEnabled withTemplate: 'Prefilled' };
    package: 'Foo'

so the developer is free to use #new, custom instance creation methods or instance creation methods, define his #printOn:/storeOn: representation of the custom slot with more freedom.

Another Benefit of option B would be that the slots name could already generated by the slot depending on an argument to the slot creation on the right side. So for instance in

Object << #Account
    slots: { #bankAccountNumber => TypedAttributeSlot modelName: 'Bank account number' type: String length: 20 };
    package: 'Money'

the custom typed attribute slot might (during instantiation/initialization) already define the slot name #bankAccountNumber by converting the model name 'Bank account number' automatically into a proper camel case slot name for Pharo:

image

astares commented 4 months ago

As the above example demonstrate the current way we deal with them is confusing and does not "feel natural" anymore as one has to detailed follow the processing order to see what is happening. One requires more time to explain the differences, one runs into trouble if unary messages should be used to parameterize the slot instance, ...

But Pharo is intended to make things more easy and "natural" right from the first look.

Slots are very useful and can lead to less boilerplate code written if applied properly. In my private meta research projects I use them a lot and they provide many benefits.

I tried to work around the above problems by using an own binary method #--> that does not create the slot instance but accepts on the right side the already created initialized slot. But then I run into issues with the class builder and other in Calypso as I do not follow the tooling expectation to use #=>

And sorry for this lengthy post - but I really would like to see the slot definition to be more presented in an obvious and more clear way. This also gives more confidence in how the slots are designed/need to be written and provides some consistencies amont the method variantions one can use.

Ducasse commented 4 months ago

I agree with you. #people => ToManyRelationSlot inverse: #x inClass: #SlotExamplePerson is super confusing. I always thought that inverse:... was send to ToManyRelationSlot.

Thanks for showing that.

MarcusDenker commented 4 months ago

My main motivation was to avoid having to put () everywhere.

I will check the suggestions. We need to do a pass anyway, as we have to add some way to decorate Slots with a nice way in addition. This gives us the opportunity to improve this even for the case where we do not decorate.

astares commented 4 months ago

I understand the motivation - but without the () you will have the trouble of the mixed message priorities leading to the described issues and confusion.

@MarcusDenker Thanks for checking!

astares commented 1 month ago

@MarcusDenker Any update on this one?

MarcusDenker commented 1 month ago

Not yet, too much other things in June/July and then there was holidays...

astares commented 1 month ago

@MarcusDenker Take your time. I used part of my holiday to think about this one seeking for a better solutions - but discarded them. Using natural message style is the most flexible IMHO even when it would mean to go with the additional braces.

It would also be more logical and natural to the developer as it boils down to the usual objects and messages again. Nothing special to learn additionally. Basically it say: you want a slot then name it and write something to create it.

Might also help when more tool support is available as left style is just a symbol and right side a creational message/expression following regular syntax, ...