AppliedGo / comments

Utteranc.es comments for appliedgo.net
Creative Commons Zero v1.0 Universal
0 stars 0 forks source link

flow2go #15

Open christophberger opened 1 year ago

christophberger commented 1 year ago

Written on 03/11/2017 20:10:30

URL: https://appliedgo.net/flow2go/

christophberger commented 1 year ago

Migrated comment, written by Samuel Lampa on 03/13/2017 12:52:32

Thanks for a really nice writeup, which I hope will help popularize FBP approaches in the Go community!

Just some notes about things I have found useful in my own "framework-less FBP" experimentation:

For merging, I found it useful (in the spirit of FBP best practices, I think), to create a re-usable merger component, which I just plug in to the network where needed ... so that I don't need to write this slightly more complicated code more than once. I got such component in scipipe for example: https://github.com/scipipe/... (I should really move this out to http://flowbase.org , where I thought to gather these kind of very generic and re-occuring components).

The other note is about the "Process" function. I found it neat (got hinted to it by twitter.com/egonelbre ) to not include the go keyword `inside` the "Process()" function (Well, I call them "Run()", but same same), but instead prepend that when calling the method instead. This allows to skip to prepend it for the last process, which enables to drive the whole network without need of pulling from a "Done" channel at the end. .... so, one would go:

go proc1.Process()
go proc2.Process()
proc3.Process()

Well, clearly a matter of taste of course :) ... but it also enables to let the process (or network) component do this trickery for you ... so you can just do:

net := NewNetwork()
net.AddProcesses(proc1, proc2, proc3)
net.Process()

This is perhaps more of a matter of taste though, but I tend to think the code gets a slight bit cleaner that way :)

The best example of how code following these recommendations would look, is the network definition of a small tool to convert from semantic RDF data to MediaWiki XML pages, that I wrote:
https://github.com/rdfio/rd...

All in all, I guess all of this is more or less a matter of taste and preferences, but I thought I'd share my 5c fwiw! :)

christophberger commented 1 year ago

Migrated comment, written by Christoph on 03/13/2017 14:20:03

Hi Samuel,

Thanks for the detailed feedback.

I put the "go" keyword inside the Process() methods as I assumed there would be no occasion where a processing node can or should run in the main goroutine. Your approach proved my assumption wrong :-) This is indeed a good way to simplify the code (although I claim that my code is not too bad in terms of readability :) ).

-Christoph

christophberger commented 1 year ago

Migrated comment, written by Samuel Lampa on 03/13/2017 14:40:53

> I put the "go" keyword inside the Process() methods as I assumed there would be no occasion where a processing node can or should run in the main goroutine

I assumed so too, until I saw Egon Elbre put the go-keywords outside (in his thread answer here: https://groups.google.com/f... ), which I hadn't even thought of, and then realized it could fix the "driving" problem to just omit it for the last one ... that the network has to be driven from the main go-routine somehow.

> This is indeed a good way to simplify the code (although I claim that my code is not too bad in terms of readability :) ).

True, not a very big difference at all :)

christophberger commented 1 year ago

Migrated comment, written by Christoph on 03/14/2017 15:53:03

> it could fix the "driving" problem to just omit it for the last one

Yes, this is indeed a pattern worth keeping in mind.

Thinking about it, one aspect in favor of the "go-keyword inside" approach is that the caller is not bothered with choosing which functions to prepend the go keyword to. Rather, the caller just needs to call Process() (or run() or whatever) on all nodes. It's similar to the net/http style of starting goroutines implicitly.

christophberger commented 1 year ago

Migrated comment, written by Oliverpool on 04/06/2017 11:06:19

Hi,
really nice article.

I think you could simplify a bit this (repeated) part:


go func() {
for {
sentence, ok := <-wc.Sentence
if !ok {
fmt.Println("WordCounter has finished.")
close(wc.Count)
return
}
wc.Count <- &count{"Words", len(strings.Split(sentence, " "))}
}
}()

by iterating on the channel:


go func() {
for sentence := range wc.Sentence {
wc.Count <- &count{"Words", len(strings.Split(sentence, " "))}
}
fmt.Println("WordCounter has finished.")
close(wc.Count)
}()

And then using the comment of @samuellampa:disqus :


func (wc *wordCounter) Process() {
fmt.Println("WordCounter starts.")
for sentence := range wc.Sentence {
wc.Count <- &count{"Words", len(strings.Split(sentence, " "))}
}
fmt.Println("WordCounter has finished.")
close(wc.Count)
}
// ... later...
go wc.Process()

And now the `Process` method is short and very easy to read!

christophberger commented 1 year ago

Migrated comment, written by Christoph on 04/06/2017 12:06:38

Thanks for sharing this! Indeed, the original "for - comma, ok - if" construct is not the most concise one. I am not sure why I did not think of just ranging over the channel :-)

Regarding the placement of the `go` keyword, as I mentioned in the other comment thread, there are pros and cons to both approaches. Personally I would prefer to spawn the goroutines inside the methods, so that developers that use these functions are freed from the mental burden of having to remember where exactly to put the `go` keyword. Forget to use them, and the net would not run. Forget one of them, and the net might behave weird. Forget to leave out the last one, and your main routine exits before the net even has started.

It's like a car's transmission. With automatic transmission, you let the car decide when to shift gears. No way that you can select the wrong gear or even stall the engine. Manual transmission gives you more control over the engine but you have to know how to move the lever and engage the clutch properly.

christophberger commented 1 year ago

Migrated comment, written by Oliverpool on 04/06/2017 12:38:43

Good point.
Maybe one could do like `cmd.Run/Start` (https://golang.org/pkg/os/e... ), with Start() just spawning the goroutine go self.Run().

christophberger commented 1 year ago

Migrated comment, written by Christoph on 04/06/2017 14:54:14

Hmm... but the go keyword already spawns a goroutine, so why another Run() or Start() method?

christophberger commented 1 year ago

Migrated comment, written by Oliverpool on 04/06/2017 15:41:34

So that you have the "automatic transmission": wc.Start();lc.Start()...
and the "manual transmission": go wc.Run(); go lc.Run()... (Run being actually the Process method)

christophberger commented 1 year ago

Migrated comment, written by Christoph on 04/06/2017 16:27:16

Ok, now I see, you want to offer the user both options! That's certainly the most comfortable solution (although perhaps at the expense of simplicity).
So many options to choose from... :)

christophberger commented 1 year ago

Migrated comment, written by Phil Ruggera on 07/22/2017 18:00:49

Yes, fewer choices is key to managing complexity as the program gets larger. Nicely done with flow2goWithInterface! I'd like to explore further the use of FBP.

christophberger commented 1 year ago

Migrated comment, written by Samuel Lampa on 10/24/2017 12:58:53

Yes, I agree in general. But there is often the problem that you have to drive the processing chain by sending or receiving from the main go-routine somehow. That is, go-routines will generally not start just because they are thrown off, unless something is forcing them to get going, via channels.

Of course, this might depend a bit on the approach taken, but I had this problem in scipipe/flowbase at least, which was solved this way.