Page MenuHomePhabricator

Simple factories (with or without model)
Open, HighPublic

Description

Related to T5301.
This task is mostly a question for now.

We have a few legacy APIs that require a callback to create a single object, based on some input data or string.
Examples: drag & drop (for the floating image object), textblock items (emoticons), ...

I honestly couldn't figure out how to create an image factory for those. Efl.Ui.Image.Factory requires a Model. I have no idea how to create a Model at the moment, and this may look overkill for something like textblock. Definitely overkill for copy & paste.

Thoughts?

Yes, the factory design currently in tree is for use with model, but we indeed need to have a general pattern for a factory. My current concern is that it seems that we might end up with a factory for containers widget that are not relying on model (menu, toolbar, ...) that would not be usable outside of that pattern. Maybe we can figure out a factory design for both model and non model container that would work, for now we don't. So maybe let's implement both and we will look at what we have once it is done and merge them back if possible afterward.

@cedric @jpeg
I am not much familiar with the factory pattern well, but one thing worried is that "how reference guide would be generated well for application developers".

Let't think about the NEW toolbar with this factory.
Whenever, developers wanna make item similar things to insert in their toolbar, there should be good guideline for that -
but I cannot imagine how factory things can be described well in the reference for toolbar eo class.

I think this could be thought together when we think about applying factory-pattern to various usage of EFL.

Let't think about the NEW toolbar with this factory.
Whenever, developers wanna make item similar things to insert in their toolbar, there should be good guideline for that -
but I cannot imagine how factory things can be described well in the reference for toolbar eo class.

My current idea is to do one example factory at least that cover existing use case. This could be the basis of the documentation I think.

In T5567#88773, @cedric wrote:

Let't think about the NEW toolbar with this factory.
Whenever, developers wanna make item similar things to insert in their toolbar, there should be good guideline for that -
but I cannot imagine how factory things can be described well in the reference for toolbar eo class.

My current idea is to do one example factory at least that cover existing use case. This could be the basis of the documentation I think.

Ok. Then, Should we inform this to Daniel Kolesa ? Or each developer need to keep in mind everytime they make their factories ? OR - factory would be managed by special person ?

I think this is one of the "important + new" concept changes for EFL developers.

jpeg added a comment.Jun 12 2017, 8:20 PM

Ok. Then, Should we inform this to Daniel Kolesa ? Or each developer need to keep in mind everytime they make their factories ? OR - factory would be managed by special person ?

Obviously this can't be a "special person" doing all the factories.

I think this is one of the "important + new" concept changes for EFL developers.

The point here is we need a simple API where it is easy to implement the "factory_create" method. Be it using efl_override (I don't like it as it is right now) or inheritance (very cumbersome in C) or using another method it must be easy to do.
Right now we have not defined such an API. I don't have a great idea yet, which is why I opened this ticket.

From what I have looked, our current code base as two different pattern where factory will be usefull. The first one is with MVVM and how we generate view. This is the current factory we have in tree. The second case are for toolbar, menu and stuff a like. Where you have limited number of item that are defined by some set of string (icon, label) and a callback. That is the second type of factory I see. I guess I should go at converting maybe toolbar rather sooner than later to use that new pattern. At the end, factory is something that an application can write themself and that would enable them to override the possibility of this container, but inside elementary I don't think there will be that many different type of factory. Maybe just an icon + label is enough.

There might be more cases for factories in EFL. However, I think it is important not to mix things up. The current factory is model specific because it is to be used to create something from the information contained in a model node _and_ update information on that something (usually a widget or a canvas) when information from the model changes asynchronously.

Factory is a very useful pattern, so it is likely that it will appear in more places in EFL than only where models are used or useful, but we shouldn't force this model factories to work well where models are not used. We should probably create a new interface for factory where model is not used and some other kind of information will be used to create elements.

BTW, we might get away with factories in some places by using function pointers in Eolian. Our C# branch already has this implement in Eolian, I'm rebasing it over master upstream.

So, instead of getting a Eolian object with a create function in it, just pass a function pointer with whatever signature the method wants and that returns the element it will create. Like this:

-- pseudo code, not real eolian --
function factory_function {
  params {
    a: string;
    index: int;
  }
  return: efl.ui.Canvas;
};

class Toolbar {
  methods { @property item_factory { set {} params { factory: factory_function; } } }
}

And then, Toolbar just calls factory_function whenever it needs a new efl.ui.Canvas to show
as an element of the toolbar.
jpeg added a subscriber: q66.Jun 13 2017, 7:40 PM

Would function pointers work in all languages? I know @q66 was being very careful about them at first...
If we get function pointers, this would definitely make EO simpler, but I still think we should be careful to not have too many different callback signatures anyway.

q66 added a comment.Jun 14 2017, 4:19 AM

@jpeg they won't work unless we figure out lifetime and memory management tracking... if you have ideas, feel free to suggest

Guys, maybe I got the wrong message, but the Model kind of factory for menu and toolbars fit perfectly.

What I keep repeating is that if Model is cumbersome to create and use (and I truly believe that's the case, as I exposed many times), then we must improve Model, including more model sources... for instance why not create helpers that populate a Model from textual description? This matches TEXTBLOCK use case as there is a standard markup... of course we could do something similar to menu, maybe using XML, YAML or something like that, where we can easily extend (people are saying label + icon, but I'm pretty sure things will come, such as type (checkbox, radio... which in turn needs group) and key binding accelerators (when we implement such thing).

I agree with you on the fact we want menu/toolbar creation to be defined by text. Now, I am not convinced that using data model for this is a good idea at all. The added complexity doesn't seems to bring that much benefit to me (Except we would have only one family of factory), but I am pretty sure we can implement toolbar/menu like list is being implemented as a view. I am willing to explore more this possbility, but I would like to reduce our NIH if we provide a textual syntax for defining menu/toolbar, so does anyone know of any usable XML, JSON menu/toolbar definition that already exist and we could parse that as a data model input.

Why using data model for this is not a good idea? In theory it is simple, light and easy to use... or it should, if not we're doing it wrongly... actually given the number of items, only "ease of use" matters...

Remember that the menu can be offloaded to a different UI, like the Ubuntu's protocol over D-Bus that allows MacOS-X like (and also MacOSX...), so making it a model is reasonable. This also make a standard approach to activation (callbacks -> events), hierarchy and specializations such as checkbox (booleans) or radio...

Think as an user: I need to create a menu of items: add items and children, listen for activation, set current state (checkbox, radio). then I need to create a navigation using lists of items: add items and children, listen for activation, set current state (checkbox, radio). If they look the same, why shouldn't them be the same???

In fact in many cases you may want to interchange the presentation, consider:

  • desktop app will use a menu bar -> drop down menus
  • desktop app will use toolbar and exceeding icons collapsed as "more" icon that presents a drop down menu
  • mobile app will use an overlay naviframe + list instead of dropdown

Then all you need to change is the factory that returns views for those items... that's all. The information/structure is the same.

Function pointers will work. There's absolutely no problem at all with lifetime in the developed version. The method gets three parameters in the place of a function pointer: The function pointer itself, a void* private data, a function pointer to free the void* private data. That is all that is needed.

BTW, I'm not against a textual model representation. It seems quite interesting actually and can benefit our widget implementation since we can focus on view implementations instead of naked widgets. The complexity might decrease performance, however.

Function pointers will work. There's absolutely no problem at all with lifetime in the developed version. The method gets three parameters in the place of a function pointer: The function pointer itself, a void* private data, a function pointer to free the void* private data. That is all that is needed.

it won't work with eolian and bindings, that's what @q66 is trying to remove... how do you automatically generate those in many bindings? There is no context... so be gone void* and callbacks.

@barbieri They are generated for C, ofc. Not for C#, C++, lua etc. They use the void* and free function to be able to pass state for functions.

jpeg added a comment.Jun 15 2017, 7:17 PM

I opened a ticket for function pointers in T5583. Please move that dicussion there.

I am not against models per se, but right now I can't figure out how to use them for something like toolbar. What matters most is ease of use. An XML or whatever storage format would help but the API itself also needs to be dead simple, so I can easily add/remove items programmatically.

Note also that in the case of textblock the <item> things can be anything, so the user should be able to easily set up her own factory function.

In my mind toolbar, menu or list should be the same like this:

Eo *container, *item;

container = efl_model_container_new(); // consider something that holds children

item = efl_model_item_new(); // consider something that keeps properties
efl_model_property_set(item, "label", "File");
efl_model_property_set(item, "icon", "file");
efl_model_child_add(container, item);

// ... repeat ...

elm_toolbar_model_set(tb, container);
elm_menu_model_set(mn, container);
elm_list_model_set(ls, container);

would show the same model as a toolbar, menu and list.

felipealmeida added a comment.EditedJun 16 2017, 5:31 AM

There are some models that use Eina containers. But for menus we could even simpler, maybe?

Efl_Model* model = efl_menu_model_new
  ("File -> label: File, icon: file\n"
   "Open -> ... \n");

It is yet another language, which sucks, maybe json?

Regards,

Felipe, I believe that this is good, just make it a standard format and usable in general not just for menus, but any structure. Example:

Efl_Model *model = efl_model_from_json("[{'label': 'File', 'icon': 'file'}, ....]");

Efl_Model *model = efl_model_from_xml("<item label='File' icon='file'/>....");

Efl_Model *model = efl_model_from_yaml(
" - label: File\n"
"   icon: file\n"
"...");

IOW all that matters is the structure, they have children, each with some properties. All of these helpers could take schemas to further validade and use proper types, otherwise just fallback to strings and use eina_value conversions when something else is desired.

In my mind toolbar, menu or list should be the same like this:

Eo *container, *item;

container = efl_model_container_new(); // consider something that holds children

item = efl_model_item_new(); // consider something that keeps properties
efl_model_property_set(item, "label", "File");
efl_model_property_set(item, "icon", "file");
efl_model_child_add(container, item);

// ... repeat ...

elm_toolbar_model_set(tb, container);
elm_menu_model_set(mn, container);
elm_list_model_set(ls, container);

would show the same model as a toolbar, menu and list.

This is exactly what I think is not a simplification. Today you do add an item in one line (passing icon, label and a select callback function). It isn't making things simpler if you have to call 4 more functions and his exactly my current concern.

Felipe, I believe that this is good, just make it a standard format and usable in general not just for menus, but any structure. Example:

Efl_Model *model = efl_model_from_json("[{'label': 'File', 'icon': 'file'}, ....]");

Efl_Model *model = efl_model_from_xml("<item label='File' icon='file'/>....");

Efl_Model *model = efl_model_from_yaml(
" - label: File\n"
"   icon: file\n"
"...");

IOW all that matters is the structure, they have children, each with some properties. All of these helpers could take schemas to further validade and use proper types, otherwise just fallback to strings and use eina_value conversions when something else is desired.

I am not a fan of this solution either as it has no type checking and you have to independently specify the callback to be triggered which is likely going to be a source of error. I don't have a proper solution that would use model and view at the moment. Will see if I can find some idea, but I am sure you understand my concern here.

dude, this is a problem with helpers, not with model. Of course you can create a function that does that, like specifically for menus/toolbars:

item = efl_model_menu_item_new(container, "File", "file");

Or you make it easier overall, which is my preference, something like:

item = efl_model_props_new(container, "label", EFL_MODEL_STRING("File"), "icon", EFL_MODEL_STRING("file"));

here it's a PITA from C that you can't get the va_args type for free, then either that or some printf-like string. But it has the benefit that it's immediately usable with other stuff, like to populate a list, a music player, etc.

In T5567#89083, @cedric wrote:

Felipe, I believe that this is good, just make it a standard format and usable in general not just for menus, but any structure. Example:

Efl_Model *model = efl_model_from_json("[{'label': 'File', 'icon': 'file'}, ....]");

Efl_Model *model = efl_model_from_xml("<item label='File' icon='file'/>....");

Efl_Model *model = efl_model_from_yaml(
" - label: File\n"
"   icon: file\n"
"...");

IOW all that matters is the structure, they have children, each with some properties. All of these helpers could take schemas to further validade and use proper types, otherwise just fallback to strings and use eina_value conversions when something else is desired.

I am not a fan of this solution either as it has no type checking

read the part I said about schema, all of these guys have associated schemas that provide type and structue checking... and these will be used anyway, at least if we want models, those are already the most common formats as well as from SQL... recall REST uses XML or JSON, GraphQL uses JSON...

and you have to independently specify the callback to be triggered which is likely going to be a source of error.

why is that? I thought we'd get events from view on activated/selected items or such... no? when you load a list of music from SQL, do you need to register a callback for each item so you "play on click"? It's the same solution.

I don't have a proper solution that would use model and view at the moment. Will see if I can find some idea, but I am sure you understand my concern here.

I do, but I think it's being short sighted as a music library, a file structure or a menu are just different views of similarly structured models. It should be clear from Enlightenment's menu, recall you have EFM with list/grid views, but you can also navigate files from desktop menu...

We just need a convention for views on what properties are, like "icon". Then for a file model you use an overlay/proxy that will take a real model based on filesystem and then inject a new property "icon" that is either a fixed icon based on mime-type or a thumbnail...

I don't think we can do va_args, but we can do arrays I guess. In C, we could have some helpers for that. With the eo function being :

menu = efl_model_array_properties_new(container, EFL_MODEL_ARRAY("label", EFL_MODEL_STRING("File"), "icon", EFL_MODEL_STRING("file"));

And the helper macro being :

menu = efl_model_properties_new(container, "label", EFL_MODEL_STRING("File"), "icon", EFL_MODEL_STRING("file"));
In T5567#89083, @cedric wrote:

Felipe, I believe that this is good, just make it a standard format and usable in general not just for menus, but any structure. Example:

Efl_Model *model = efl_model_from_json("[{'label': 'File', 'icon': 'file'}, ....]");

Efl_Model *model = efl_model_from_xml("<item label='File' icon='file'/>....");

Efl_Model *model = efl_model_from_yaml(
" - label: File\n"
"   icon: file\n"
"...");

IOW all that matters is the structure, they have children, each with some properties. All of these helpers could take schemas to further validade and use proper types, otherwise just fallback to strings and use eina_value conversions when something else is desired.

I am not a fan of this solution either as it has no type checking

read the part I said about schema, all of these guys have associated schemas that provide type and structue checking... and these will be used anyway, at least if we want models, those are already the most common formats as well as from SQL... recall REST uses XML or JSON, GraphQL uses JSON...

We are talking about menu and toolbar here, right ? Is it a real normal use case to get this populated from a GraphQL, XML or whatever ? If we are talking about MVVM in general, I do agree, but that is a different discussion. Here the discussion is how to make data model easy to use in the scenario of toolbar and menu becoming a view of it.

and you have to independently specify the callback to be triggered which is likely going to be a source of error.

why is that? I thought we'd get events from view on activated/selected items or such... no? when you load a list of music from SQL, do you need to register a callback for each item so you "play on click"? It's the same solution.

The list as a realize event that give you the created object that you can setup callback on. As I said at some point above, this is moving to a more complex solution as it require to go fully dynamic for something that I think the majority of use case is fully static.

I don't have a proper solution that would use model and view at the moment. Will see if I can find some idea, but I am sure you understand my concern here.

I do, but I think it's being short sighted as a music library, a file structure or a menu are just different views of similarly structured models. It should be clear from Enlightenment's menu, recall you have EFM with list/grid views, but you can also navigate files from desktop menu...

We just need a convention for views on what properties are, like "icon". Then for a file model you use an overlay/proxy that will take a real model based on filesystem and then inject a new property "icon" that is either a fixed icon based on mime-type or a thumbnail...

I see your point here, but we should not forget that the goal is to make it easy for doing application. Your proposal is for sure the most flexible, but I am still to be convinced that it will be easy to use.

In T5567#89086, @cedric wrote:

I don't think we can do va_args, but we can do arrays I guess. In C, we could have some helpers for that. With the eo function being :

menu = efl_model_array_properties_new(container, EFL_MODEL_ARRAY("label", EFL_MODEL_STRING("File"), "icon", EFL_MODEL_STRING("file"));

And the helper macro being :

menu = efl_model_properties_new(container, "label", EFL_MODEL_STRING("File"), "icon", EFL_MODEL_STRING("file"));

sure, also the EFL_MODEL_STRING() and the likes are better written as Eina_Value stuff, then we do not create yet-another struct, enum, parse, release...

and following your array, it would result in an array of struct { const char *name, const Eina_Value value }, then you get all info you need and it's easy to consume.

In T5567#89097, @cedric wrote:
In T5567#89083, @cedric wrote:

Felipe, I believe that this is good, just make it a standard format and usable in general not just for menus, but any structure. Example:

Efl_Model *model = efl_model_from_json("[{'label': 'File', 'icon': 'file'}, ....]");

Efl_Model *model = efl_model_from_xml("<item label='File' icon='file'/>....");

Efl_Model *model = efl_model_from_yaml(
" - label: File\n"
"   icon: file\n"
"...");

IOW all that matters is the structure, they have children, each with some properties. All of these helpers could take schemas to further validade and use proper types, otherwise just fallback to strings and use eina_value conversions when something else is desired.

I am not a fan of this solution either as it has no type checking

read the part I said about schema, all of these guys have associated schemas that provide type and structue checking... and these will be used anyway, at least if we want models, those are already the most common formats as well as from SQL... recall REST uses XML or JSON, GraphQL uses JSON...

We are talking about menu and toolbar here, right ? Is it a real normal use case to get this populated from a GraphQL, XML or whatever ? If we are talking about MVVM in general, I do agree, but that is a different discussion. Here the discussion is how to make data model easy to use in the scenario of toolbar and menu becoming a view of it.

I'm saying that we will need the JSON, XML, YAML model constructors for other purposes. And with them we can use their own schema validation, so we do get validation/type-checking as you wish "for free".

I'm not saying "we'll define that menus will be implemented in JSON", I'm saying that we implement menus using Efl_Model and that may come from whatever produces that: JSON, XML, YAML, SQL, REST, GraphQL or explicit API calls. Then our "menu/toolbar" specific bits turns to be only the properties and structure, nothing else.

and you have to independently specify the callback to be triggered which is likely going to be a source of error.

why is that? I thought we'd get events from view on activated/selected items or such... no? when you load a list of music from SQL, do you need to register a callback for each item so you "play on click"? It's the same solution.

The list as a realize event that give you the created object that you can setup callback on. As I said at some point above, this is moving to a more complex solution as it require to go fully dynamic for something that I think the majority of use case is fully static.

talking at IRC we came to the conclusion that given our selection and activation states will be maintained in "proxy" models, then we just make the proxy model an actual interface with more explicit events. That is: we will have an selected event in addition to property changed, plain simple and consistent, making not just menu/toolbars easy to use, but also lists, trees...

of course there is an alternative that is the view to emit the selected items. And these could coexist, maybe be implemented on top of another.

I don't have a proper solution that would use model and view at the moment. Will see if I can find some idea, but I am sure you understand my concern here.

I do, but I think it's being short sighted as a music library, a file structure or a menu are just different views of similarly structured models. It should be clear from Enlightenment's menu, recall you have EFM with list/grid views, but you can also navigate files from desktop menu...

We just need a convention for views on what properties are, like "icon". Then for a file model you use an overlay/proxy that will take a real model based on filesystem and then inject a new property "icon" that is either a fixed icon based on mime-type or a thumbnail...

I see your point here, but we should not forget that the goal is to make it easy for doing application. Your proposal is for sure the most flexible, but I am still to be convinced that it will be easy to use.

it must be easy to use otherwise model won't be used... think of list, we do not want a distinction like "elm_list x elm_genlist" where people said "ah, genlist is stupidly complex to use, let's use list because it's easier". We should avoid users having to do such choice based on ease of use, they all should be the same and very easy to use.

I have to agree with @barbieri here that using proxy models as source for events is a very MVVM-thing to do. Maybe we should discuss how we can make proxy MVVM models an easy thing to write?

cedric renamed this task from [Question] Simple factories without model to Simple factories without model.Jun 16 2017, 2:38 PM
jpeg renamed this task from Simple factories without model to Simple factories (with or without model).Jun 19 2017, 7:26 PM
cedric triaged this task as High priority.Jul 10 2017, 3:30 PM

@cedric
So, will this task get the answer after finishing to define MVVM things ?
Or, any other idea on this ?

@cedric @felipealmeida

ping for remind ~ !!
This is the one of the most important tasks, since many widget classes will use this concept together.

jpeg added a comment.Oct 12 2017, 1:27 AM

Edje.Object's item_provider may use something like this as well. Alternatively only use a specific function pointer (this is the easy way, for now).

bu5hm4n moved this task from Backlog to primitives on the efl board.Jun 10 2018, 12:32 PM
bu5hm4n moved this task from primitives to Backlog on the efl board.
zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:51 AM
bu5hm4n edited projects, added efl: widgets; removed Restricted Project.Jun 11 2018, 9:11 AM
zmike added a subscriber: zmike.

A #Goal ticket should not be set to a milestone.