Fl_Flex method name question

[melcher....@googlemail.com]

Is `int set_size(Fl_Widget *w) const;` correct, or should it be `int get_size(Fl_Widget *w) const;` with a 'g'. Since it's `const`, it's only returning something, and not setting anything.

[Gonzalo Garramuño]

El 24/10/22 a las 14:12, 'melcher....@googlemail.com' via fltk.coredev escribió:

Is `int set_size(Fl_Widget *w) const;` correct, or should it be `int get_size(Fl_Widget *w) const;` with a 'g'. Since it's `const`, it's only returning something, and not setting anything. --

It should probably be neither.  A better name for it would be:

bool check_size(Fl_Widget* w) const

as it does not return the value of the widget size but checks whether there's a value set for it.

[Bill Spitzak]

Or hasSize()?

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/31831a38-157e-90be-1d69-60a4f4fb4178%40gmail.com.

[Gonzalo Garramuño]

El 24/10/22 a las 14:33, Bill Spitzak escribió:
> Or hasSize()?
Yes, has_size() is a better name for it.  Don't let *me* design an API
please!

[melcher....@googlemail.com]

After coding around a bit, it returns 1 if a child widget is using it's w() or h() as a fixed size, but still flexible in position, or 0 if Flex positions and resizes the widget. So here is my list: `is_fixed`, `has_fixed_size`, well, I don't know any more. Native speakers?

[Albrecht Schlosser]

On 10/24/22 19:12 'melcher....@googlemail.com' via fltk.coredev wrote:

Is `int set_size(Fl_Widget *w) const;` correct, or should it be `int get_size(Fl_Widget *w) const;` with a 'g'. Since it's `const`, it's only returning something, and not setting anything.

Yes, it is returning whether `void set_size(...)`has been called on this widget. The naming follows (ehm, I tried to follow) the good old FLTK convention that the setter and getter methods have the same name.

https://www.fltk.org/doc-1.4/classFl__Flex.html#a3235849fc0e99512d76c0048de68e6a8

Setter(s):

void Fl_Flex::set_size(Fl_Widget &w, int size)
void Fl_Flex::set_size(Fl_Widget *w, int size)

Getter:

int Fl_Flex::set_size(Fl_Widget *w) const


I admit that the "set_" prefix of the setter method makes this confusing because the getter doesn't set anything. I'm open for better suggestions, and since FLTK 1.4.0 has not yet been released we can IMHO still change this if we decide to do so.

BTW, for history: in the original contribution the setter was SetSize() and there was no getter method.

Returning an int rather than bool is always FLTK convention.

As far as I have seen proposals, here are my comments:

hasSize(): -1 (violates FLTK conventions)

has_size(): naming is OK, but IMHO too general. Although it corresponds to set_size(), my gut feeling is that this is confusing as well (every widget has a size, right?).

check_size(): similar to has_size(), IMHO too general.

is_fixed(), has_fixed_size(): these are my personal favorites so far, the latter more than the former.

is_fixed_size() would IMHO be another good option.


Are there more proposals? Votes?

[Albrecht Schlosser]

On 10/25/22 02:49 'melcher....@googlemail.com' via fltk.coredev wrote:
> After coding around a bit, it returns 1 if a child widget is using
> it's w() or h() as a fixed size, but still flexible in position, or 0
> if Flex positions and resizes the widget.

Hmm, why not look at the documentation, wouldn't this be easier than
"coding around a bit"? ;-)
SCNR. ;-)


https://www.fltk.org/doc-1.4/classFl__Flex.html#a3235849fc0e99512d76c0048de68e6a8

"Return whether the given widget has a fixed size or resizes dynamically."


If this is not enough we can be more explicit, like saying that
`set_size(Fl_Widget *widget, int size) has been applied.

[Greg Ercolano]

On 10/25/22 08:12, Albrecht Schlosser wrote:

is_fixed_size() would IMHO be another good option.

    That paints the clearest picture in my mind,
    if I understand this correctly.

    But I'm thinking has_size() works well; it's short, easy to remember.

    Also kinda self documenting when used in code I think, esp. in the
    context of use, e.g.

        if ( flex->has_size() ) {
            ..do fixed size stuff..
        }

[melcher....@googlemail.com]

I have been "coding around" a little bit more and implemented what I think is a decent Fluid GUI for Fl_Flex. Please check out https://github.com/fltk/fltk/pull/523 .

Sorry for being so late to the game after Albrecht already released the code, particularly because I checked the code much earlier, but in retrospect wasn't focussed enough. So here are my suggestions for an API, but this will probably break existing code.

So, Albrecht, if you want to keep the API, I am ok with that too.

We have `margin(int, int, int int)` and `margins(int*, int*, int*, int*)`, one to set and one to retrieve all margins, and `margin()` to get the left margin, and `margin(int, int=0)` to set all margins equally and optionally the gap. Shouldn't we either use the singular `margin` for all cases, or use `margins(int, int, int int)` instead of `margin(int, int, int int)`?

We have `set_size(Widget*)`, `set_size(Widget*,int)`, and `set_size(Widget&, int)`. `int` has a double meaning here. If it's 0, don't use a fixed size, else be fixed. My suggestion is `int fixed(Widget*) const`, `void fixed(Widget*, int yes)`, and `void fixed_size(Widget*, int size)`. This takes away the special meaning of size 0. The naming seem more explanatory to me, and it follows the FLTK rule that getters and setters have no get and set prefix.

The first form returns 0 or 1 if a widget is flexible or fixed.

The second form sets a widget fixed to its current size. This is neat because the constructer requires a width and height anyway, so just use that.

The third form can set a fixed size of 0 as well, because `fixed(w, 0)` can make a widget flexible again.

And finally, I would like a `int size(Widget*)` that returns `w()` or `h()`, depending on `Fl_Flex::horizontal()`.

Thanks for reading.

[Albrecht Schlosser]

On 10/25/22 19:10 'melcher....@googlemail.com' via fltk.coredev wrote:
> I have been "coding around" a little bit more and implemented what I
> think is a decent Fluid GUI for Fl_Flex. Please check out
> https://github.com/fltk/fltk/pull/523 .

Yeah, I've just seen it and I looked at the diffs regarding fixes in
Fl_Flex. I'm fine with adding comments and your fixes look plausible as
well, thanks for finding these. I'll review and comment the PR further
with details.

I need some time to check the fluid functionality though ...

> Sorry for being so late to the game after Albrecht already released
> the code, particularly because I checked the code much earlier, but in
> retrospect wasn't focussed enough. So here are my suggestions for an
> API, but this will probably break existing code.

I believe it's still OK to break the API if it's worth it because the
API gets improved. As soon as FLTK 1.4 will be released we need to keep
it, but FLTK 1.4 is not yet released, so everything is still in the flow...

> So, Albrecht, if you want to keep the API, I am ok with that too.

I'll check your suggestions later, I just wanted to let you know for
now. It's been a while since I ported Fl_Flex to FLTK (based on
https://github.com/osen/FL_Flex) and I need to refresh my memory...

[API suggestions removed for now, I'll comment these later.]

> Thanks for reading.

Welcome.

Thank you for writing these valuable comments.

[Albrecht Schlosser]

Sorry for the late reply, I've been too busy with other stuff to pursue the Fl_Flex questions.

On 10/25/22 19:10 'melcher....@googlemail.com' via fltk.coredev wrote:

Sorry for being so late to the game after Albrecht already released the code, particularly because I checked the code much earlier, but in retrospect wasn't focussed enough. So here are my suggestions for an API, but this will probably break existing code.

So, Albrecht, if you want to keep the API, I am ok with that too.

I was reluctant to break existing code, but since Fl_Flex is not yet *officially* released I'm willing to change the API for consistency and better compatibility with FLTK coding style.

I believe I implemented most of your suggestions, see comments below and PR #594.
https://github.com/fltk/fltk/pull/594

We have `margin(int, int, int int)` and `margins(int*, int*, int*, int*)`, one to set and one to retrieve all margins, and `margin()` to get the left margin, and `margin(int, int=0)` to set all margins equally and optionally the gap. Shouldn't we either use the singular `margin` for all cases, or use `margins(int, int, int int)` instead of `margin(int, int, int int)`?

I decided to use singular for all cases. I chose the plural form in an early development stage because I used other parameter types, but this is no longer necessary. Done in the PR.

We have `set_size(Widget*)`, `set_size(Widget*,int)`, and `set_size(Widget&, int)`. `int` has a double meaning here. If it's 0, don't use a fixed size, else be fixed.

That's correct, it describes how it is (was).

My suggestion is `int fixed(Widget*) const`, `void fixed(Widget*, int yes)`, and `void fixed_size(Widget*, int size)`. This takes away the special meaning of size 0. The naming seem more explanatory to me, and it follows the FLTK rule that getters and setters have no get and set prefix.

OK, this is more complex.

The first and simple part: I renamed all set_size(…) methods to fixed(…). I believe that's OK.

The first form returns 0 or 1 if a widget is flexible or fixed.

"First form" = `int fixed(Widget*) const`

This should be fixed by renaming (see PR).

The second form sets a widget fixed to its current size. This is neat because the constructer requires a width and height anyway, so just use that.

"Second form" =`void fixed(Widget*, int yes)`

The "charm" of Fl_Flex is that you can create widgets with new Widget(0,0,0,0). This doesn't need a size at all.

I didn't implement this and left this form with the meaning to set the size with the second argument:

Renamed form: `void fixed(Widget*, int size)`

This has still the meaning that size = 0 (re)sets the widget to flexible. But see below.

The third form can set a fixed size of 0 as well, because `fixed(w, 0)` can make a widget flexible again.

Third form = `void fixed_size(Widget*, int size)`

OK, that would be a new "feature": setting a widget to fixed size 0. With the current API we can't do that.

I hesitate to add another method with a similar name. I'd rather leave just the method name 'fixed()'.

If we wanted to make it possible to set a widget to fixed size 0 then we could change the meaning of the size argument such that '-1' (a negative value) sets the widget to flexible rather than 0.

Is there a compelling reason to set a widget to fixed size 0 in an Fl_Flex widget? I can't imagine one.

And finally, I would like a `int size(Widget*)` that returns `w()` or `h()`, depending on `Fl_Flex::horizontal()`.

We can do this but I didn't implement it yet. One problem with this is that the widget size is only valid after the final layout is calculated. This could be achieved by the user by calling layout() or resize() before calling size(Widget*).

Do we really need this? The user (program) should know which value it's interested in and can call the widget method w() or h() directly. Do you have a specific case in mind?

That said, I'm not sure if the following is a good decision, but I decided to push my changes to a new branch Fl_Flex and opened PR #594 for further discussion and review.
https://github.com/fltk/fltk/pull/594

Thanks for reading.

Welcome. I read this a long time ago, thanks for considering my late reply.

[Gonzalo Garramuño]

El 13/12/2022 a las 01:10 p. m., Albrecht Schlosser escribió:
>
> That said, I'm not sure if the following is a good decision, but I
> decided to push my changes to a new branch Fl_Flex and opened PR #594
> for further discussion and review.
> https://github.com/fltk/fltk/pull/594

I am already using Fl_Flex (old) in my code.  When you are done with the
changes, would it be too much to ask to add some test of it so I can
look at the new API.

[Albrecht Schlosser]

On 12/15/22 22:04 Gonzalo Garramuño wrote:
>
> I am already using Fl_Flex (old) in my code.  When you are done with
> the changes, would it be too much to ask to add some test of it so I
> can look at the new API.

No problem, it's all available. I merged the mentoned PR, so the new API
is now online. But beware, it's not final until 1.4.0 is released,
although I'm pretty confident that there are no more changes pending.

If you used the old ("original") Fl_Flex then the main change you need
to do is change setSize() to fixed() and isSetSize() to fixed() as well
(FLTK style: setter and getter have the same name).

I just uploaded the docs: https://www.fltk.org/doc-1.4/classFl__Flex.html

... and current docs on the GitLab mirror are updated daily anyway (wait
until tomorrow morning (~7 hours from now) for the next update!):
https://fltk.gitlab.io/fltk/classFl__Flex.html

A minimal example program is included in the docs and there are two
examples in 'test/flex_demo.cxx' and 'test/flex_login.cxx'. Another one
is 'examples/howto-flex-simple.cxx'.

Maybe there are some new features in the original 'osen/FL_Flex' github
repo since I copied it but there are also new features in the FLTK
version (e.g. margin and gap).

Have fun! All feedback would be appreciated.