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 :-)
> +_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
