Data Binding Validation

[AlbinoGeek]

Summary

If data binding is meant to be the single source of truth, I would say it needs inline validation.

The Get and Set operating on error makes me feel this could happen within the current API.

Problem

Currently, as the intention is that you load data binding values from preferences (?) -- the only safety that binding grants you is type-conformity. (A binding.String will always return a string.)

However, there is no requirement to match a given format, length, etc (as a developer/program may have.)

This leads to a situation where corrupt preferences can load into and cause a program to crash in an unrecoverable fashion.

Again, under the assumption that the bound variables were the single source of truth.

Possible Solutions

What if bindings had validators? It would only take adding one function and one line of code per binding.

str := binding.NewString()
str.SetValidator(func(s string) error {
  if len(s) < 4 {
    return errors.New("value too short")
  }
})
if err := str.Set("foo"); err != nil {
  panic(err) // value too short
}

positive := binding.NewInt()
positive.SetValidator(func(i int) error {
  if i < 0 {
    return errors.New("must be positive")
  }
})
if err := positive.Set(-1); err != nil {
  panic(err) // must be positive
}

Then, bound widgets could simply pass through the validator on their binding.

E.g:

e := widget.NewEntryWithData(str)
e.SetText("bar")
if err := e.Validate(); err != nil {
  panic(err) // value too short
}

e.Bind(binding.IntToString(in))
e.SetText("-1")
if err := e.Validate(); err != nil {
  panic(err) // must be positive
}

[AlbinoGeek]

Perhaps the widget validator pass-through should be its own issue, as that represents a usability change.

[andydotxyz]

Setting the Entry will automatically trigger bind actions too..

if err := e.Validate(); err != nil {
  panic(err) // value too short
}

Is not needed, it will execute anyway

[AlbinoGeek]

Running e.Validate() will return the error returned from binding.Set() ?

[andydotxyz]

Why do you need to run e.Validate() manually? If an error occurs in validation within the bindings it will be flagged in the Entry using the usual validation output.

[AlbinoGeek]

I call e.Validate() manually when checking whether the user should be able to Save the configuration or not. (this is a settings dialog.)

func (settings *paneSettings) validate() error {
    if err := settings.apiKey.Validate(); settings.apiKey.Text != "" && err != nil {
        return fmt.Errorf("invalid value for \"API Key\": %v", err)
    }

    if err := settings.replaysRoot.Validate(); err != nil {
        return fmt.Errorf("invalid value for \"Replays Root\": %v", err)
    }

    if err := settings.updatePeriod.Validate(); err != nil {
        return fmt.Errorf("invalid value for \"Check Every\": %v", err)
    }

    return nil
}

I would imagine the exact same scenario occurs with a Form which should call its contents validators and enable/disable the Submit button based on them.

[andydotxyz]

Form uses Entry.SetOnValidationChanged so it does not have to call anything manually.

[andydotxyz]

The Get and Set operating on error makes me feel this could happen within the current API.

Yes, the API has been designed with this in mind. Any data binding can be a validator, or provide one, or accept an optional one. Bindings can be chained so there is no need to extend the API to support what is requested in this issue.

[andydotxyz]

Perhaps this is a feature request for a builtin validation constructor? Such as:

binding.ValidatedString(String, fyne.StringValidator) String

[AlbinoGeek]

https://github.com/AlbinoGeek/sc2-rsu/blob/fyne/develop/cmd/BoundString.go

That was what I ended up having to do, yeah. That would do allow for as easy an implementation as the examples in the opening post, yes.

[Bluebugs]

Perhaps this is a feature request for a builtin validation constructor? Such as:

binding.ValidatedString(String, fyne.StringValidator) String

Does that mean we should introduce all the validator type to cover all the data binding type at this time for this task?

[andydotxyz]

It would need to be added for all the types with validation, which is a much shorter list than that of all the types of binding we support.