Skip to content

Fixing allPropertiesDo properties duplications#119

Closed
AntoninGoslin wants to merge 7 commits into
moosetechnology:developmentfrom
AntoninGoslin:fixing_allPropertiesDo_duplications
Closed

Fixing allPropertiesDo properties duplications#119
AntoninGoslin wants to merge 7 commits into
moosetechnology:developmentfrom
AntoninGoslin:fixing_allPropertiesDo_duplications

Conversation

@AntoninGoslin

Copy link
Copy Markdown
Contributor

I fixed the way allPropertiesDo is working to avoid properties duplication.

We now build properties, traits and superclass in allProperties and not in allPropertiesDo.
We are using a dictionnary to avoid duplications.

I removed the duplicated allPropertiesDo: method from FM3Class. It now inherits the behavior from FM3Type.

allPropertiesDo now using allProperties himself.

Closes #118

@Gabriel-Darbord Gabriel-Darbord left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The usual pattern is having one method to stream the things (Fame properties here), and another method to build a collection using the stream.
That's how it worked before, with allPropertiesDo: and allProperties respectively.

This PR proposes to inverse the pattern: allProperties builds the collection, then allPropertiesDo: uses it to stream and then discards the collection.

That would be a net loss in terms of performances.
Isn't there a way to keep the pattern while fixing the issue?

@jecisc

jecisc commented May 26, 2026

Copy link
Copy Markdown
Member

The usual pattern is having one method to stream the things (Fame properties here), and another method to build a collection using the stream. That's how it worked before, with allPropertiesDo: and allProperties respectively.

This PR proposes to inverse the pattern: allProperties builds the collection, then allPropertiesDo: uses it to stream and then discards the collection.

That would be a net loss in terms of performances. Isn't there a way to keep the pattern while fixing the issue?

I was saying the same things to Antonin IRL :)

I am wondering if we still need the performances. The only user in Moose is in FMFutureProperty but Benoit is working on removing the need of the future properties.

If we want to keep the optimization we need to keep a table of the visited properties in #allPropertiesDo:

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.

allPropertyDo duplicating some properties

3 participants