cvui icon indicating copy to clipboard operation
cvui copied to clipboard

Suggestion: cvui::checkbox() returns state change instead of current state

Open vestri opened this issue 6 years ago • 4 comments

Hello, Currently :

cvui::checkbox() returns a boolean value that indicates the current state of the checkbox: true if it is checked, or false otherwise.

It might be better if cvui::checkbox() returns true if checkbox has changed as for other elements (trackbar, button...). The status can be obtained by the pass boolean variable theState.

Thanks Nice tool.

vestri avatar Nov 28 '18 09:11 vestri

Hi! That's a great addition. I am sad I haven't thought about this from the very beginning, because changing this now would implicate a compatibility break.

Your suggestion is very likely to be incorporated into the 3.0 release though. A major version bump makes this compatibility break (and any other) very evident.

Dovyski avatar Nov 28 '18 15:11 Dovyski

Hi Sorry to interfer, but I have a suggestion that could also improve the usage

// cvui.h
namespace cvui
{
  struct checkbox_r 
  {
    bool changed;
    bool checked;
  }
 checkbox_r checkbox(const cv::String& theLabel, bool *theState, unsigned int theColor = 0xCECECE)
}

// usage
if ( cvui::checkbox(frame, 200, 150, "Checkbox", &checked).changed ) 
  // handle the change

// or 
auto r = cvui::checkbox(frame, 200, 150, "Checkbox", &checked);
if (r.checked)
  // ...
if (r.changed)
  // ...

}

This has the adavantage that, while it is still a breaking change, it will be caught by the compiler, whereas changing the content of the return value without changing the return type will lead to runtime error much later.

This coud also be applicable to some other widgets such as counter:

  // cvui.h
  struct counter_r 
  {
    bool changed;
    int value;
  }
  counter_r counter(cv::Mat& theWhere, int theX, int theY, int *theValue, int theStep = 1, const char *theFormat = "%d");

  // usage
  if (counter(&nb).changed)
    // handle change

As an aside, may be those alternate versions could be put into a separate namespace in order to avoid breaking compatibility ( cvui_alt is the only (bad) name that came to my mind )

pthom avatar Nov 28 '18 16:11 pthom

@pthom thank you for the ideas! Even though I like the concept of the compiler catching the compatibility break error, returning a struct (or object) seems a bit too complicated for me. It is not simple enough according to the "cvui phylosiphy".

Having the function in separate namespaces could also help, but I am afraid it will generate a bit of confusion among developers.

Dovyski avatar Nov 29 '18 07:11 Dovyski

This is an absolutely essential feature for me. It is confusing that the checkbox behaves differently to the other components in this respect.

anselanza avatar Dec 29 '20 16:12 anselanza