Hello,
On Thu, Oct 26, 2017 at 9:30 PM, Gustavo Sverzut Barbieri <
barbi...@gmail.com> wrote:
> On Thu, Oct 26, 2017 at 10:12 AM, Amitesh Singh <amitesh...@samsung.com>
> wrote:
>
> thanks for the improvement, but one hint I said before went unnoticed
> and would save great deal of this implementation:
>
>
> > +typedef struct
> > +{
> > + const char *template;
> > +} Efl_Ui_Format_Data;
>
> this is already stored in the object, due format_string_set... that's
> why I'm saying that you should pass it to the callback, so there is no
> need to store it again, providing a free_cb to release that.
>
>
>
> > +_default_format_cb(void *data, Eina_Strbuf *str, const Eina_Value value)
>
> here you should pass: str, value AND template :-)
>
>
I don't think there is a need to pass tempate here since its least useful
to user and its a burden for anyone to have both strbuf and template string.
We got another API for passing string template.
> > +_default_format_free_cb(void *data)
>
> then this is not needed.
>
>
>
> > +EOLIAN static void
> > +_efl_ui_format_format_string_set(Eo *obj, Efl_Ui_Format_Data *sd,
> const char *template)
> > +{
> > + if (!template) return;
> > + eina_stringshare_replace(&sd->template, template);
> > + efl_ui_format_cb_set(obj, sd, _default_format_cb,
> _default_format_free_cb);
>
> because here you'd not call "efl_ui_format_cb_set()"... you'd do that
> once at the constructor/init, then the callback gets the template
> stored by efl_ui_format_string_set().
>
>
> > - elm_layout_text_set(obj, "elm.units", buf);
> > + sd->format_cb(sd->format_cb_data, sd->format_strbuf, val);
>
> this is not that good. You should wrap this into a protected method
> that implementers call, like:
>
> efl_ui_format(obj, strbuf, val);
>
> Think of bindings (ie: python/javascript/c#), they may want to call
> the implementation of efl.ui.slider.... how to? you need to provide a
> method to allow that.
>
>
> --
> Gustavo Sverzut Barbieri
> --------------------------------------
> Mobile: +55 (16) 99354-9890
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
