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

[Enh]: Missing Evaluation API & Tests #14439

Open
wants to merge 2 commits into
base: Pharo12
Choose a base branch
from

Conversation

seandenigris
Copy link
Contributor

I know adding value and cull API can be a controversial rabbit hole (e.g. #value:value:value:value:value:value:value:value:value:), but of the two methods I'm proposing, the first will greatly simplify Magritte packaging and the second makes Symbol polymorphic with all the other similar receivers. I hope that you will consider them. I even wrote tests, lol!

  • Symbol>>#cull:cull - used in Magritte and will make packaging much easier
  • Symbol>>#valueWithEnoughArguments - already implemented by Object, Block, and MessageSend; make polymorphic with those

- `Symbol>>#cull:cull` - used in Magritte and will make packaging much easier
- `Symbol>>#valueWithEnoughArguments` - already implemented by Object, Block, and MessageSend; make polymorphic with those
seandenigris added a commit to seandenigris/Pharo-Enhancements that referenced this pull request Aug 5, 2023
@jecisc
Copy link
Member

jecisc commented Aug 5, 2023

I’d love that!
I have an equivalent of #cull:cull: on symbols in at least 5 of my projects! And to avoid conflicts I end up with methods such as #mdlCull:cull:, tlCull:cull: and so on

@jecisc
Copy link
Member

jecisc commented Aug 5, 2023

I approved but I let open to have other opinions here

@Ducasse
Copy link
Member

Ducasse commented Aug 6, 2023

I do not like. I do not see the added value.

valueWithArguments: aCollection [ 
	^ aCollection first perform: self withArguments: aCollection allButFirst.
]

@Ducasse
Copy link
Member

Ducasse commented Aug 6, 2023

I find it confusing.

(#asNumber valueWithArguments: #('1'))

I do not see the point of creating an array to place the receiver in, just for the sake of executing it.

1 perform: #asNumber 

@guillep
Copy link
Member

guillep commented Aug 7, 2023

@seandenigris @jecisc it would be nice if you better document the usage. I understand there is a need, it would be good for the community if you tell us what are examples of usages!

@seandenigris there are no such messages in object :)

@Ducasse I understand you don't like it, I personally have no strong opinion. But if there is a community need, that's important too. My main arguments here are:

  • people are already doing this! In their own custom packages with extension methods! So acknowledging it is better to take optimization (positive or negative) decisions in the future for example.
  • It's not symbols on themselves that harm static analyses, it's dynamically crafted symbols. But one could handcraft a lot of things in our language. Forbidding is not in the spirit :)
  • I don't think there are any runtime performance implications (actually, blocks are slower than symbols right now)
  • Maybe these ugly extensions could be packaged separately and not in Kernel, to allow unloading and making dependencies explicit?

This said I'm personally for more minimalistic APIs: less to maintain, easier to work with. But if we go full that way I'd like to clean a lot of collection methods.

@seandenigris
Copy link
Contributor Author

I find it confusing.

(#asNumber valueWithArguments: #('1'))

Yes! We are on the same page. When you know how many arguments you have and need, this would not be used.

The only reason that I know of to use this somewhat convoluted method is to have a generic way to plug together loosely coupled components. This sort of thing is often needed in Magritte for example.

Here's an example... Let's say I have an accountsSource in my model. I'd like to be able for a user to provide the actual needed object, or a lazily evaluated one e.g. a block. However, blocks can get tricky in Magritte because they are not easily serializable and some users serialize entire Magritte meta models. So now, you have to do something like pass a symbol there representing a message to send to the described object returning the accountsSource.

So now, we have #cull: and hopefully very soon #cull:cull: (in this PR) to support this, but there is also a generic mechanism for any number of arguments to prevent needing #cull:cull:cull:cull:cull:cull:cull:cull:cull:cull:cull:cull:cull:cull:cull:cull:. This generic mechanism, that already exists in Pharo, is #valueWithArguments:. It exists for all other "message like" evaluatable things - Block, MessageSend - and Object itself.

Screenshot 2023-08-07 at 12 33 19 PM

The only problem is that there is one missing implementation - Symbol. Once that is added, one can supply an Object or any common evaluatable in a case such as the one above.

Also, please note that I'm not suggesting this be used in Pharo Core, only that it be provided along with the other implementors.

Please let me know if that was helpful or if there's any other info I can provide.

@Ducasse
Copy link
Member

Ducasse commented Aug 8, 2023

This is ok. I understand the point with the block API. I never like the hijack of symbol as a block.
I do not like either cull:.
Pharo is bloated and I would prefer to have less than more reflective features.

@jecisc
Copy link
Member

jecisc commented Aug 8, 2023

This is ok. I understand the point with the block API. I never like the hijack of symbol as a block. I do not like either cull:. Pharo is bloated and I would prefer to have less than more reflective features.

Personally I find the code way more readable using the symbols than a block that will just send one unary message.

classes collect: [ :class | class name ]

vs

classes collect: #name

When I read it, with the second I have the impression of reading english while the first one gives me the impression to read code. And the block is taking more time to my brain to understand (It's not a lot, but it still impacts).

As Guille said, it's not to use in Pharo. We can have a rule of not accepting that in Pharo itself. But for external projects it would make thing much more easier.

@jecisc
Copy link
Member

jecisc commented Aug 8, 2023

@guillep The use cases are to let the users configure some objects.

I have for example one MDL component that hold a selected object in it and the user can configure a tooltip based on this object. So we want the user to me able to just write

selectWidget tooltip: #description

But! in some edge cases, the user whats to customize way more things and in that case he needs the canvas of Seaside to do something such as:

selectWidget tooltip: [ :selectedItem :canvas | canvas bold: selectedItem description ]

Because of this edge case, either we cannot use symbols (but it makes the code in the framework more readable) or we need to check the type of the parameter and the number of arguments ourself. So it's easier to add our own #cull:cull: in Symbol.

I had this case in multiple of my projects.

@Ducasse
Copy link
Member

Ducasse commented Aug 8, 2023

I have plenty of examples showing that using a symbol instead of a block totally obscure the code because we have no way to know if this is a symbol or a disguided block. I do not want to get started there.
To me this is really bad.

@seandenigris
Copy link
Contributor Author

This discussion is interesting!

Given that:

  1. we are not proposing using these messages in Core, and
  2. Other variants/implementors already exist in Core

I'm wondering if the opposition is to the PR, which aims to provide needed functionality for external libraries and their users, or just an interesting academic discussion?

@guillep
Copy link
Member

guillep commented Aug 9, 2023

I'm wondering if the opposition is to the PR, which aims to provide needed functionality for external libraries and their users, or just an interesting academic discussion?

Hi @seandenigris I don't think there is a need to be condescending either.

I understand there is no need to use it in the kernel. On the other side, controlling that no users are introduced will be an active job for maintainers doing code reviews, writing rules, tests, running those in the CI...

I think the discussion is whether the cost of having it inside (and having to maintain it, optimize it, and so on) is worth the gain you're going to have. In that sense, maybe it would be nicer to have a small project in pharo-contributions that holds all these and other extensions. We can even move the extraneous one from Pharo there. Then magritte, material design lite, and others could depend on that if needed.

@Ducasse
Copy link
Member

Ducasse commented Aug 9, 2023

I'm not doing academic discussions here. I'm trying to understand

  • why I do not like an addition (getting over the mere feeling)
  • foresee the impact when people will use it in every place (because people will use features just blindly and it will take us ages to recover, let us look at Smalltalk everywhere and other niceties),
  • think in terms of how we can reduce reflective usage instead of just systematically increasing it
  • get code that can be read nicely (I want code that I can read without having to read the definition of the message I sent and the use of Symbol as blocks obscure this)
  • get code that does not break any future possibilities to build (limited) static analyzer such as type inferencers.

Now my personal take:

  • I do not like cull: because it is reflective, it makes people write sloppy code (like in the glamour builder where it was impossible to know what was the number of expected arguments of a block and all was just magic and hateful),
  • I do not like the use of symbols for blocks because suddenly it makes the reader impossible to know exactly what is happening without reading the definition of the method it is passing the symbol to - I can write code that you will never be able to guess correctly if a symbol is a block or not and I really think that a language is to like people write good code not sloppy one. It breaks the static nature of blocks. A block is a block and we know it statically.

So sorry to be slow to think.

@Ducasse
Copy link
Member

Ducasse commented Aug 9, 2023

I have a question why can we use perform

[:x :y| ...] perform: #value:value: withArguments: #( 2 3)

there you can mix symbols and blocks.

1 perform: #+

@Ducasse
Copy link
Member

Ducasse commented Aug 9, 2023

Is a MessageSend object not better? I think that we should remove the load on Symbols.

@seandenigris
Copy link
Contributor Author

Hi @seandenigris I don't think there is a need to be condescending either.

That was not my intention. I’m sorry if it seemed that way. I sincerely was asking whether the discussion was related to accepting this PR or more generally about these techniques, because this is not a new feature. Perhaps academic was a poor word choice, since it can have negative connotations.

I have a question why can we use perform

Not at a computer, but I think because it’s not implemented by Object. A common use case is to allow either lazy evaluation (e.g. a block) or the actual object you want.

MessageSend is an interesting idea. Let me play with that

@seandenigris
Copy link
Contributor Author

For my own education if you will indulge me…

  • I do not like cull: because it is reflective, it makes people write sloppy code (like in the glamour builder where it was impossible to know what was the number of expected arguments of a block and all was just magic and hateful),

Isn’t that more of a documentation issue? It is common for a block to be passed in one place and used in another. Unless there is a method comment explaining the arguments, one is lost

  • I do not like the use of symbols for blocks because suddenly it makes the reader impossible to know exactly what is happening without reading the definition of the method it is passing the symbol to - I can write code that you will never be able to guess correctly if a symbol is a block or not and I really think that a language is to like people write good code not sloppy one.

In that regard, what is the difference between anObject xyzSource: [ :e | e doSomething ] and anObject xyzSource: #doSomething?

It breaks the static nature of blocks. A block is a block and we know it statically.

That’s interesting. I don’t know about that. What are the ramifications?

@Ducasse
Copy link
Member

Ducasse commented Sep 3, 2023

In fact I would like to remove Object>>valueWithArguments: because I do not understand why it is there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants