Page MenuHomePhabricator

[MVVM] Changes to Efl Data Model
Open, Incoming QueuePublic

Description

Cedric made huge changes to EFL when bringing the new Eina Futures to EFL. This means that some efl models got broken, and the EFL Model interfaces got changed.

Right now, EFL Data Models have two ways to get a property value: property_get returning a future _AND_ through properties,changed event.

This has two problems:

First and foremost: Every user of the efl data model needs to do property_get for properties _AND_ listen to changes through an event.
This is bad and redundant.

Also, keeping information updated is a pain because it must deal with Eina_Value all the time and you can't pass the responsibility of keeping the information updated to other parts of the code.

So, I was thinking in using this new interface for Efl_Model:

  • Observable_Value.eo --
class Observable_Value (Efl.Object)
{
  methods {
    value @property {
       values {
          value: any_value;
       }
    }
    write_to {
      params {
        @in type: ptr(Eina_Value_Type);
        @in ptr:    void_ptr;
      }
    }
    events {
      value,changed,before;
      value,changed,after;
    }
}

interface Efl.Model ()
{
   @property property {
       keys {
          name: stringshare;
       }
       values {
          value: Observable_Value;
       }
   }
}

This would allow us to modify Efl.Ui.Layout to accept Observable_Value and in part_set. So, we could just property_get and we could just pass Observable_Value.

We could also create helpers that call methods passing the value of the Observable_Value everytime the value changes. So, not everything needs to understand Observable_Value directly:

In C:

EFL_OBSERVABLE_VALUE_CALL (1, &efl_ui_text_set, obj, obsvalue);

Which would register with obsvalue event and call efl_ui_text_set as long as obj is still valid, when obj or obsvalue is deleted, then the memory is freed and, if needed, the event is unregistered in obsvalue.

liFac.CreateEvent += (object, model, parent, style) {
if (style == "default") {
  var item = (Efl.Ui.ListDefaultItem) object;
  item.icon = imgFac.Create(model, object);
  item.label. model.PropertyGet("label");
}
else if (style == "title") {
  var item = (Efl.Ui.ListTitleItem) object;
  item.icon = imgFac.Create(model, object);
  item.label = model.PropertyGet("label");
}}

so you meant this propertyGet returns observable value and label will detect the value changes and set the text internally?

I like this idea that makes more simple on the user side code. how much time do you think to make this features and apply the models?

Here I brought you some sample code of applying multi style factory in the view.

class Program
{

static void EflMain()
{
    string resDir = Directory.GetCurrentDirectory()+"res/";

    Eio.Model mdl = new Eio.Model();
    mdl.SetPath(resDir);

    // Set Composite Model for selection property
    Efl.ModelCompositeSelection selMdl = new Efl.ModelCompositeSelection();
    selMdl.SetModel(mdl); 

    Efl.Ui.ListView lView = new Efl.Ui.ListView(parent = mFrame); //guess mFrame is already exist
    lView.SetModel(selMdl);

    Efl.Ui.ListItemMultiFactory lFac = new Efl.Ui.ListItemMultiFactory(parent = lView); //what is the role of this parent
    lFac.SetItemClass(Efl.Ui.ListDefaultItem.GetClass(), "item_class", "default");
    lFac.SetItemClass(Efl.Ui.ListTitleItem.GetClass(), "item_class", "title");

    lView.SetFactory(lFac);

    Efl.Ui.ImageFactory imgFac = new Efl.Ui.ImageFactory(parent = lView); //what is the role of this parent
    imgFac.SetModel(selMdl); // Sometimes... they might want to use another model.. then how we can give proper model child info in the create events? merge the model to final one?
    imgFac.Connect("filepath");

    // when the view call the create, it is just future and when style property is ready,
    // actual efl_add and createEvent is called. 
    liFac.CreateEvent += (object, modelChild, parent, style) {
        if (style == "default") {
            var item = (Efl.Ui.ListDefaultItem) object;
            item.icon = imgFac.Create(modelChild, object);
            item.label = modelChild.PropertyGet("filename");
        }
        else if (style == "title") {
            var item = (Efl.Ui.ListTitleItem) object;
            item.icon = imgFac.Create(modelChild, object);
            item.label = modelChild.PropertyGet("filename");
        }
    }

    lView.SelectEvent += (object, modelChild, parent) {
        var item = (Efl.Ui.ListItem) object;
        Console.WriteLine("item: " + item.GetIndex + " is selected");
    }
}

}

is there anything wrong?
please let me know anything not what you think in the code.

so, with this samples. I want to share some opinions and problems which we worried about.

  1. Efl.ModelComposite ~ : this model composite name looks not proper as our naming rules of classes,

    like, Efl.Ui.Factory -> Efl.Ui.ItemFactory, Efl.Ui.MultiItemFactory...

    the classifying name(Item, Composite) need to be prefix not postfix of type(Factory, Model) of class.

    Efl.CompositeModel, Efl.SelectionCompositeModel looks more natrual and valid name for me. what do you think about it?
  1. Model / ProxyModel / View/ Factory.... what is role of parent? : in current code, parent set with class new looks useless, it may only have some meaning about it's life cycles.... nothing else. we need to define what is the parent and guide what will happen after parent set to the user.

    I think some parent set may remove API calls like SetModel or SetFactory.. if it need to be manually called like currently, the model and view, factory and it's parent need to have some definition and roles clearly.
  1. What is meaning of SetModel in Factory? : Item Factory wasn't need SetModel cause it internally set model by the view, but imageView need to set Model. here is the thing. what is the necessity of SetModel() in the factory if we call create function manually without SetModel() and connect(). we can pass the modelchild and property name with create function manually,

    item.icon = imgFac.Create(modelChild, object, "filepath");

    and set Model in the factory makes confusing model and view and factory relationships complexly. which place is proper place of factory in diagrams? between model and view? or view side only? This looks unclear to us.
  1. SetItemClass : this setItemClass() is very simply works. but I wonder we could support more complex case of classifying styles with condition by this way. sometimes, they only need very simple condition to set the style without making one more extra model property(like odd and even) and sometimes, they may need more complex condition compares. how we could support this kind of demands? I think we could give condition function like sort or filter does. what did you think about it?
  1. label is ObservableValue? : item.label = modelChild.GetProperty(~ if we return the Observable_Value, then how we can support SetText on this label? sometimes they might do not need model property to setText and this Item can also be used for Simple List. how we can support SetText and ObserableValue both in one Part of item?
larryolj added a comment.EditedNov 1 2018, 7:07 AM

@SanghyeonLee

I will try clarify some points

  1. Efl.Ui.ImageFactory doesn't have setModel, factories doesn't have a model, only a item created by factory

    if the item is a efl_ui_layout_object you can do:
imgFac.connect("", "filepath")
and in createEvent:
item.modelConnect("icon", imgFac); //efl_ui_factory_model_connect
item.connect("label", "filename"); //efl_ui_model_connect

you can see more in "layout_model_connect.c" example
running a example with ./layout_model_connect </path/to/a/directory/with/some/image/>

hmm... that was previous way and I'm talking about the way of using Efl.Part in Efl.Ui.List_Item with factory create().
this was discussed with @felipealmeida in the last meeting at August.... I'm worried about this conversation was not progressed or shared with you....

and without SetModel... connect looks more weird to me.
what is connected with factory? factory doesn't know about model, but how he can connect with some data?
it doesn't looks clear to user side what is "Connect" and what will happen if I "Connect some strings".
that is why we already want to have some instance or types rather than strings.

  1. OK
  1. The role of parent is just for lifetime, you don't need it in C#.
  1. No need to SetModel to factory because the Create called by (which generates the createEvent) receives the model as parameter.
  1. We could support inheritance from simplel multifactory where the factory cand o whatever it wants.
  1. Can't we do that by SetContent? SetText would continue to be the same.
SanghyeonLee added a comment.EditedNov 8 2018, 4:09 AM
  1. Okay.
  2. you meant item factory or image factory? or both of them? If in both case, I think we can remove setModel from factory fully.
  3. yes. so the our idea is basic one and.. if some other requirements comes, let's expand it more.
  4. Okay. not bad actually.

So... is it all good of the code that I wrote?

  1. Both

Yes, I think it is all good. We just would need to modify a little bit the parts generation for C#, allow it to use overload on the property set so it can call SetContent/SetText.

We need to discuss deadline. When should this be ready? I'm thinking this year still, right?

Regards,

Yes..actually deadline is now being more urgent, cause there are some plans to using this feature soon.
at least basic factory need to be ready till end of this month and multistyle factory and the other stuff also need to be ready before the end of the year.
I, also have to finish gridview and grid factory beyond this work,
and some Tizen specific class and visual effect need t be ready in deadline so that should be my work also.

so I hope,
[Nov]

  • working code of Simple Factory and Listview with Composite Model(class, interface and method fix)

[Dec]

  • working code of Multiple Factory and ListView and other model features
  • working code of Gridview
  • working code of Tizen specific features.

It would be very appreciate that you update your own deadline or update plans in each tasks.
if you want to split some task, welcome to make another task.

cedric added a subscriber: cedric.Nov 14 2018, 4:34 PM

This seems to be a work around MVVM pattern. The idea of MVVM is that all the decision logic is put into a ViewModel that act as a proxy for the View. This ViewModel would be were you make the choice for the style for example. If you move the decision making out of the MVVM pattern, you are breaking the automatic chain that was there and you can't build automatic test that do not rely on UI for example as the proper information is not exposed inside a VM property.

Also observable are full size Eo object, this will result in a very heavy implementation in term of memory use.

zmike moved this task from Backlog to Felipe on the efl: mvvm board.Jan 9 2019, 12:25 PM