Depends on D9953
Details
Details
Diff Detail
Diff Detail
- Repository
- rEFL core/efl
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
| src/lib/efl/interfaces/efl_ui_factory.eo | ||
|---|---|---|
| 36 | @segfaultxavi please bring your seal here, this sounds weird to me. | |
| src/lib/elementary/efl_ui_widget_factory.c | ||
| 92 | This is now a warning. | |
Comment Actions
In general, I do not like method names in gerund form (building, constructing and releasing).
This is explaining what the caller was doing when the method was called, instead of telling the callee what it should do.
This is confusing, and, in fact, the calls to efl_ui_factory_building() are pretty obscure...
I suggest these APIs are replaced with standard infinitive forms (build, construct and release). If this is ambiguos, add a prefix/suffix to indicate what should be built/constructed/released. Maybe build_item?
| src/lib/efl/interfaces/efl_ui_factory.eo | ||
|---|---|---|
| 36 | Remember that lines must be wrapped at 120 columns. Release an UI object before it is invalidated. After this call, the object can be recycled to another @Efl.Ui.View. | |
Comment Actions
Can we consider removing constructing entirely (this should just be a constructor implementation), renaming building to post_finalize, and renaming releasing to recycle or something?
Comment Actions
Removing constructing will remove the ability to do things before the construction is finalized without having to reimplement both widget and factory to change constructor property default value.
Xavi the description of this function are exactly what you said. They describe what the caller is doing, because that define the context to the callee. I am not too sure any of the purposes change help as they are more confusing. release imply you would have to release the widget, which is not what the API is doing. The API is already releasing the widget, but is asking the factory to do the cleanup before the effective release of the object.
Comment Actions
Arguably all this function could be event like item,releasing on the factory if that would be easier for you.
Comment Actions
I still don't understand what you mean by this. The custom constructor can just call the super before it does its own setup, can't it? This is more or less how you're using constructing anyway.
Comment Actions
An event item, releasing would work perfectly for me. Almost all our current events are emitted after something has happened (and then they use the past tense), but event emission during the act is also common in other places (and then they use the gerund form).
But a method name in gerund form will confuse people for sure.
You have to try to come up with a name that describes what the method implementor should be doing... if it's not item_release then maybe item_cleanup, item_dispose, item_finalize... I really don't know.
| src/lib/elementary/efl_ui_widget_factory.c | ||
|---|---|---|
| 92 | Woot? | |