Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: is it a normal practices to use global non-constant variables in Fyne apps #3

Closed
dumblob opened this issue Jan 19, 2020 · 18 comments

Comments

@dumblob
Copy link

dumblob commented Jan 19, 2020

I'm quite surprised by the first counter example - namely about the way how the state is being stored and mutated. I'm pretty sure globals are not the standard practice, so I assume one would normally use a local variable in the main() scope.

But the point is, that I'd be also quite surprised if Fyne didn't provide a way to store the mutable state (in this case just an integer) in a more "reactive" manner (e.g. in some "reactive" wrapper which when written to - possibly even from a different go routine - would automagically update all the widgets which read from the "reactive" wrapper instance in their callback).

Does Fyne provide a better support for "live" data (i.e. "reactiveness") than using such plain "global" variables?

@andydotxyz
Copy link
Member

No you are quite right @dumblob the global was an oversight and has been fixed!

You'll be pleased to know that there is indeed an upcoming API for handling data in the way you describe.
I have started a branch here in preparation as we work through the examples, you can see what it will look like here:

https://github.com/fyne-io/7guis/blob/dataapi/counter/main.go
https://github.com/fyne-io/7guis/blob/dataapi/temperature-converter/main.go

@dumblob
Copy link
Author

dumblob commented Jan 20, 2020

@andydotxyz thanks for the quick reply and good news! That looks way closer to what I envisioned. Any plans for syntax sugar? I mean:

count.SetInt(count.Value()+1)

could be somehow shorter - in high-level languages it would probably be just an overloaded assignment: count += 1.

The following:

cDeg := (valueF.Value() - 32) * (5.0 / 9.0)
valueC.SetFloat(cDeg)

(from the temperature assignment)
could also be slightly simpler in an ideal world - e.g.: valueC = ( valueF - 32 ) * ( 5.0 / 9.0 )

More importantly though, does .Bind() support "composition/filtering" at the very end?

widget.NewLabel("0").Bind(count), button)

If I wanted to show value 1 is stored in count as label, I don't see currently any easier way than creating another "global" count_tmp := dataapi.NewString( "" ) and then writing:

count_tmp.SetString( fmt.Sprintf( "value %d is stored in count", count.Value() ) )
widget.NewLabel( "0" ).Bind( count_tmp ), button )

Another question would be regarding defaults - currently it seems a duplication is inevitable 😢:

count := dataapi.NewInt(0)
...
widget.NewLabel("0").Bind(count), button)

(see the 0 being twice there)
Could the API offer readout of the default value in count? I know that such pointer increases size of the struct "significantly", but it's very useful. If the struct size is of a big concern, then I'd prefer always having a second method dataapi.NewIntDefault() returning the same struct, but with the given default stored.

Wow, sorry for bloating this thread - it just popped in my mind too quickly to not write it 😉.

@dumblob dumblob mentioned this issue Jan 20, 2020
7 tasks
@andydotxyz
Copy link
Member

andydotxyz commented Jan 20, 2020

This is great feedback thanks, I will try and respond to some items and as there are things we could resolve will reopen the ticket until the design is enhanced.

Syntactic sugar
Probably not, at this stage we're not changing or adding to the language, that does not feel idomatic.
At some point on the future maybe, as it would be nice :)

Filtering (formatting?)
To do this currently you would need to add a listener to the dataapi.Int, on each change you could format your own string and set it as the text on the Label. You would not bind to the label and instead update the content with Label.SetText(). I agree that it would be nice to add something smarter - maybe we can explore this when binding to a type that supports strings. Possibly BindWithFormat(string)?

Defaults
This is a good discussion. If we follow Go's approach of having default values then NewInt() should probably take no value and we could add NewIntWithValue(int). Additionally I could have just used NewLabel("") but somehow that seems less clear

Copying in @steveoc64 as he is finalising the first dataapi at the moment. I think we should consider the defaults now, then review formatting options later as it's not the most common use-case.

@andydotxyz andydotxyz reopened this Jan 20, 2020
@dumblob
Copy link
Author

dumblob commented Jan 20, 2020

Filtering (formatting?)

To do this currently you would need to add a listener to the dataapi.Int, on each change you could format your own string and set it as the text on the Label. You would not bind to the label and instead update the content with Label.SetText(). I agree that it would be nice to add something smarter - maybe we can explore this when binding to a type that supports strings. Possibly BindWithFormat(string)?

I meant it more generic - e.g. a label showing the counter with a given offset of 99 and twice as big (whereas other places reading count.Value() would need it without the offset and without the multiplication):

count_tmp := dataapi.NewInt( 0 )
...
count_tmp.SetFloat( count.Value() * 2 + 99 )
widget.NewLabel( "0" ).Bind( count_tmp ), button )

(so it's not just about formatting - especially if the widget wouldn't eat String, but the original Int 😉 - imagine some range widgets or whatever)

@andydotxyz
Copy link
Member

Hmm, I think I see your point - but I don't know how frequently developers will want to show a value that is not representative of the value stored. The OnChanged callback can be used on the situations that people want to do something custom...

@dumblob
Copy link
Author

dumblob commented Jan 20, 2020

Hmm, I think I see your point - but I don't know how frequently developers will want to show a value that is not representative of the value stored.

It's subjective, but I use it a lot 😉. The point is, that the data are usually by far not prepared for presentation due to coupling (the data are e.g. big lists of structs or some trees or hash maps or bitmaps etc.) and one needs only certain pieces of them (e.g. just part of the picture or just two records from the huge list etc.). Therefore I called it "filtering" (or "windowing" or alike).

In other words, the raw/source data are usually not in the form the widget needs them to be (widget needs e.g. some widget-specific struct describing richt text, but the data are stored in memory and/or DB in very different formats because of space and speed constraints). This is historically one of the most painful stuff about UI - that one needs to adapt the data too much throughout the whole app just to match the last layer - UI. But that's usually nonsense and therefore there are ever-emerging new UI libs trying to solve it, but the point is, that the data should not follow UI, but UI should follow the data 😉 (i.e. first at the very last step, which is pure presentation, should be the data reordered, decoupled or (re)coupled).

@andydotxyz
Copy link
Member

Good points. However what you are currently seeing is just primitive values binding - not a map or a list. These will need to provide more complex behaviours as you say.

Our expectation is that the data source providers will have to do translation into a data descriptor that binds well to the UI. For example a database data source may exist where you filter or select and then the resulting data "map" for each item in a list would extract the data from the record to display that will then map cleanly to widgets etc.

Does this help?

@dumblob
Copy link
Author

dumblob commented Jan 20, 2020

Maybe I misunderstood, but are you describing a process which demands to change the data representation to the by Fyne required types sooner than at the place of widget declaration/definition?

If so, then no, the use case is, that one e.g. fetches data from DB(s), then works with them (incl. e.g. saving different parts of them back to DB) and then presents yet different parts of them in different parts of UI.

And it absolutely doesn't make any sense to prepare the data for the UI e.g. already during the "working with them" as it's inefficient (usually the "working with them" runs separately from UI and at different "frequencies" - i.e. UI is to reflect just some samples of all the frequent changes of the data) and demands spaghetti code style or at least increased verbosity at unrelated places.

Does that make more sense?

@andydotxyz
Copy link
Member

I think we are talking about the same thing, but it's tough to say without more concrete examples which will come with time.
At some point the conversion from data to UI output needs to occur. The proposed approach with Fyne is that an application developer would define (or load) a data source object that can translate from whatever data you have to hand into something that is GUI ready - which would then be bound to the widgets.

You can have many layers of data management behind this if you want :) I hope that what I am describing could function as the filtering you describe. What we are looking at now is the last step in presenting as they are trivial examples - data from structs etc will require an additional "layer" which is being developed as part of this API. I will try to get an example up here soon as, for example, CRUD will need that if not others.

@dumblob
Copy link
Author

dumblob commented Jan 20, 2020

I think we are talking about the same thing, but it's tough to say without more concrete examples which will come with time.

I agree. Code would be the best tool to showcase that. I'll wait until the API for structs will be sketched and then raise my hand if I'll observe something suspicious 😄.

Thanks for the great work and willingness to listen to others (that's a very valuable thing in open source!).

@andydotxyz
Copy link
Member

Great. And thank you for the input - we couldn't build it without hearing what people like or don't about the designs (both ours and that of others ;) ).

@steveoc64
Copy link
Member

steveoc64 commented Jan 20, 2020

Subbed to thread. Great thread, more more more !

Not much time this morning - coffee, then off to work, so more later.

Couple of small points worth pondering in the meantime :

  • Calling Bind() on a widget will at that point in time set the contents of the widget to match the bound data. Bind() internally calls SetText / SetWhatever on the widget to update the presentation to match the data. So its automatic.

so

count := dataapi.NewInt(100)
widget.NewLabel("").Bind(count)

Will produce a label with the "100" displayed. Inside Bind() it does call SetText() with the bound value. It does mean that the initial string passed in NewLabel() gets thrown away ... so there is some inefficiency there, but no need for redundancy.

Computed props - yep, I see what you are saying there. Keeping in mind that we are operating in a compiled environment here, so expression evaluation either needs to be added (ie - adding an expression interpreter ?), or using functions.

So something like this :

type TemperatureData struct {
  BaseDataItem  // inherit the ability to addListener, etc
  Kelvin float  // internal representation
}

func (t *TemperatureData) Celcius() float {
 return t.Kelvin + 272.15
}

func (t *TemperatureData) Farenheit() float {
  return t.Celcius() * 1.8 + 32.0
}

func main() {
  var t TemperatureData
  // couple of labels bound to func pointers as computed properties
  widget.NewLabel("").BindFunc(t, t.Celcius)
  widget.NewLabel("").BindFunc(t, t.Farenheit)
}

^^ Thats not there yet, and wont be part of the next release, but thats a general idea for computed props that dont need an expression interpreter. Would be fun to do.

Thats within reach, because BindFunc(dataItem, funcPtr) has enough info to know that it needs to register a listener on the dataItem, and enough info to know the type returned by the function, and how to convert it into the type needed for the widget (a string in this case, but it depends entirely on the widget)

There are many other properites of widgets that it would be good to bind to data values too - ie, Enable/Disable based on an expression, etc etc.

@dumblob
Copy link
Author

dumblob commented Jan 21, 2020

thats a general idea for computed props that don't need an expression interpreter.

The expression interpreter is probably not needed due to function literals ("anonymous functions" or "lambdas" or "callbacks with current values") Go supports and which can be passed "in place" to BindFunc() 😉. But let's see the demo/prerelease code first and then contemplate.

There are many other properites of widgets that it would be good to bind to data values too - ie, Enable/Disable based on an expression, etc etc.

Yep, this will be needed to be supported.

@andydotxyz
Copy link
Member

andydotxyz commented Nov 21, 2020

We now have the starts of data binding work on Fyne develop branch. Here is a snippet of how we are approaching conversion so far...

count := binding.NewInt()
widget.NewLabelWithData(binding.IntToString(count))

Hopefully that supports some of the discssions above and is certainly extensible :)

@dumblob
Copy link
Author

dumblob commented Nov 25, 2020

@andydotxyz great, that looks promising!

Do you plan to generalize it a bit (e.g. by employing synchronous programming as in S as the seemingly least friction approach)?

(take this question with a grain of salt as I didn't dive into the data API)

@andydotxyz
Copy link
Member

I think that what we are working on provides the flexibility that S is also striving for.
In a sense it is completely generic because you can implement any binding.
The example of IntToString is quite simple - it just takes a source data as a parameter, on which it registers itself as a change listener. The returned binding supports setting data and when something happens it will trigger changes back the way.
It may not be quite as small in code as S, but at the same time it is still pure Go ;)

@andydotxyz
Copy link
Member

I am going to close this issue as the version of data binding on develop branch has satisfied the open questions:

  • For formatting we support this inline, to adapt your example:
widget.NewLabelWithData(binding.IntToStringWithFormat(binding.BindInt(&count), "value %d is stored in count"))
  • regarding defaults we incorporate NewXxx alongside BindXxx, combined with widget constructor functions NewXxxWithData you can get fairly slick setup, for example a new slider that triggers change events through a zero based float binding could be
f := binding.NewFloat()
p := widget.NewSliderWithData(f)

@dumblob
Copy link
Author

dumblob commented Nov 27, 2020

Thanks @andydotxyz, that's good news. Need to try it soon after it'll be released as stabilized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants