Idea for factorizing common check (including error handling)

585 views
Skip to first unread message

mikael....@gmail.com

unread,
Mar 5, 2016, 10:54:39 PM3/5/16
to golang-nuts
Hi everyone,

I wanted to share a though I had about this kind of hot subject; maybe its worth something.

I don't think the "when" keyword can be used outside of "switch". It should be quite easy to use it to run a statement when a condition matches, in the style on defer, so we can rewrite something like

func myFunc() error {
 
var err error
 
if err = otherFunc(); err != nil {
   
return err
 
}
  x
, err = anotherFunc()
 
if err != nil {
   
return err
 
}
  yetAnotherFunc
(&err)
 
if err != nil {
   
return err
 
}

 
return nil
}

to something like

func myFunc() error {
 
var err error
 
when err != nil {
   
return err
 
}
  otherFunc
()
  x
, err = anotherFunc()
  yetAnotherFunc(&err)
 
return nil
}

The compiler can even transform the code by static analysis I think, and check err everywhere it can be changed (so, when passed by reference or on the left side of := or =).

mikael....@gmail.com

unread,
Mar 5, 2016, 11:24:02 PM3/5/16
to golang-nuts, mikael....@gmail.com
Oops... the rewrite line "otherFunc()" should be "err = otherFunc()".

Daniel Skinner

unread,
Mar 5, 2016, 11:34:13 PM3/5/16
to mikael....@gmail.com, golang-nuts
Without taking your example at face value, this is a form of data binding. Being able to do such on any arbitrary type would have nonobvious side effects. The &err example func you have highlights this. In the body of yet another func, does it automatically return after setting *err non nil? Does it need to bind as well? If it's your first time reading someone else yetAnotherFunc(*error) would you feel comfortable knowing what parts of the func body would and wouldn't run simply based on an unknown binding? What about any *type argument?

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

mikael....@gmail.com

unread,
Mar 5, 2016, 11:57:19 PM3/5/16
to golang-nuts, mikael....@gmail.com
Since the idea is to make every time the value may have been changed, I think it is safe to make the check every time the value may have been changed. And maybe forbid where non-obvious side effects may happen, since it would still be an improvement (just like map has generic and the generic case is not implemented). It could even be limited to "basic" types (bool, int, string, error?) and keep me happier :)

With this in mind, my answers to your question are:


Le dimanche 6 mars 2016 15:34:13 UTC+11, Daniel Skinner a écrit :
Without taking your example at face value, this is a form of data binding. Being able to do such on any arbitrary type would have nonobvious side effects. The &err example func you have highlights this. In the body of yet another func, does it automatically return after setting *err non nil?

It shouldn't matter, since the check is made after the function returns (just like the code in the non-when case). The idea is that the compiler add a line with the "when cond { block }" clause after every line where the support variables of "cond" are potentially changed.
 
Does it need to bind as well?

I don't think so.
 
If it's your first time reading someone else yetAnotherFunc(*error) would you feel comfortable knowing what parts of the func body would and wouldn't run simply based on an unknown binding?

It shouldn't change yetAnotherFunc. Just check the value of err after it returns.
 
What about any *type argument?

Either we also perform the check after any call involving the "when" variable or simply forbid them, and forbid assigning a "when" variable in any other way than directly with "=" .

riccard...@gmail.com

unread,
Mar 6, 2016, 10:14:35 AM3/6/16
to golang-nuts
I think there are pros & cons to evaluate here:
- [pro] obviously less code to write and maintain
- [pro] cheaper error management, would be not so far from what is happening already on other languages with classic exceptions (Java, C++, Python).
- [pro] it can make tests more effective.
- [+/-] it would make less effective stack traces because would not be clear where exactly happened a failure (same problem as C++, not Python). The stack trace must include the point where the check happened.
- [+/-] coverage would be misleading, unless tools would be adapted cutting and pasting the body of the where clause where they have been placed.
- [con] cose less fluent, the same can be already said for defer.

mikael....@gmail.com

unread,
Mar 6, 2016, 5:56:51 PM3/6/16
to golang-nuts, riccard...@gmail.com
Le lundi 7 mars 2016 02:14:35 UTC+11, riccard...@gmail.com a écrit :
I think there are pros & cons to evaluate here:
- [+/-] it would make less effective stack traces because would not be clear where exactly happened a failure (same problem as C++, not Python). The stack trace must include the point where the check happened.

I agree, the stack should include the line in the when clause and, below, the line where the support variable was changed.
 
- [+/-] coverage would be misleading, unless tools would be adapted cutting and pasting the body of the where clause where they have been placed.

I agree too, while I'd mitigate this by saying that the "when" code is actually tested at least once when marked as covered.
 
- [con] cose less fluent, the same can be already said for defer.

Yes, so we do not introduce a new problem,  we only "extend" it a bit more. I think the idea of limiting this to simple "left of =" variables would further mitigate this issue.

Rob Thornton

unread,
Mar 6, 2016, 9:03:17 PM3/6/16
to golang-nuts
As stated this is almost the opposite of a defer. It makes the code more concise but less clear.

Rather than a when keyword, something pre or preemt might work.

Preempt (func () { if err != nil { log.Print (err) } })()

This would then be called prior to any call instruction.

I still don't like it and prefer how Go is currently but it would be a more palatable alternative to your proposal for me.

mikael....@gmail.com

unread,
Mar 7, 2016, 1:07:08 AM3/7/16
to golang-nuts
Le lundi 7 mars 2016 13:03:17 UTC+11, Rob Thornton a écrit :
As stated this is almost the opposite of a defer. It makes the code more concise but less clear.

Rather than a when keyword, something pre or preemt might work.

Preempt (func () { if err != nil { log.Print (err) } })()

This would then be called prior to any call instruction.


You can't "return err" with that. It also doesn't work for the "x, err = ..." case.

mikael....@gmail.com

unread,
Mar 7, 2016, 5:14:32 PM3/7/16
to golang-nuts, mikael....@gmail.com
Le lundi 7 mars 2016 17:07:08 UTC+11, mikael....@gmail.com a écrit :
It also doesn't work for the "x, err = ..." case.

Nevermind, it works for the "x, err = " case, I though you suggested a real func call. I would rewrite rather write

preempt {

 
if err != nil {
   
return err
 
}
}

 I'm now more concerned about the idea of "calling prior to any call instruction" (in fact every statement, even "err = nil"). Maybe "preempt(err) {...}"; but a new keyword is a problem for the compatility promise I think. The "when" idea should allow more control and optimizations from the compiler, and (I hope) be compatible. From the concerns I can see, the question is how to limit the scope enough to avoid bad use.

Would a kind of "functionnal variable" be less prone to bad use? Maybe this proposition is converging a bit, in the same spirit of my original one:

var err error {
 
if err != nil { return err }
}
otherFunc
()
...

mikespook

unread,
Mar 7, 2016, 5:35:43 PM3/7/16
to mikael....@gmail.com, golang-nuts
func myFunc() error {
  
var err error
  err = otherFunc()
  x
, err = anotherFunc()
  // What will happen to the functions which are placed before `when`? 
  // Binding or not binding, it's a question.
  // The point is the scope of the keywords `when`.
  // It may take too much attention to decide how and where to use. 
  when err != nil {
    
return err
  
}
  yetAnotherFunc(&err)
  
return nil
}

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
Xing Xing (邢星)
mikespook <mike...@gmail.com>
http://mikespook.com


Florin Patan

unread,
Mar 7, 2016, 7:01:23 PM3/7/16
to golang-nuts, mikael....@gmail.com
I think this example, as well as many other show that there's a lack of understanding that errors are values: https://blog.golang.org/errors-are-values (as someone else already pointed out).
In these oversimplified examples it's show how to only "return nil" or "return err", which in some cases these are valid options but in many other cases errors should be handled and there might be code that actually can run when they happen.

package main

func readFromRedis() (data, error) {}

func readFromService() (data, error) {}

func readFromDB() (data, error) {}

func readData() (data, error) {
data, err := readFromRedis()
if err == nil {
return data, nil
}

if err.(RedisError) != redis.NotFoundError {
return nil, err
}

data, err = readDataFromService()
if err == nil {
return data, nill
}

if er, ok := err.(ServiceError); ok && er != service.NotAvailableError &&
err.(ServiceError) != service.NotFoundError {
return nil, err
}

return readFromDB()
}

func main() {
readData()
}


Can you please write this so that it handles correctly all the cases I have above with (any) new syntax / error handling? The code is a simplified version of an actual production running code.


Thank you.

mikael....@gmail.com

unread,
Mar 7, 2016, 10:39:11 PM3/7/16
to golang-nuts, mikael....@gmail.com
FYI I read the "error are values" quite some time ago ;-) Maybe with more precision on what understanding I lack could help.

Your example doesn't have a lot to factorize, but the logic seems to be "return the first successful read", so it's more about factorizing the success case. And maybe it's not what you want. You should probably re-read the examples in "errors are values" on how to factorize this :)

func readData() (data X, err error) {
   
when err == nil {

       
return data, nil
   
}

    data
, err = readFromRedis()
   
if err.(RedisError) != redis.NotFoundError {
       
return nil, err
   
}

    data
, err = readDataFromService()


   
if er, ok := err.(ServiceError); ok && er != service.NotAvailableError &&

mikael....@gmail.com

unread,
Mar 7, 2016, 10:42:06 PM3/7/16
to golang-nuts, mikael....@gmail.com
Le mardi 8 mars 2016 09:35:43 UTC+11, mikespook a écrit :
func myFunc() error {
  
var err error
  err = otherFunc()
  x
, err = anotherFunc()
  // What will happen to the functions which are placed before `when`? 
  // Binding or not binding, it's a question.
  // The point is the scope of the keywords `when`.
  // It may take too much attention to decide how and where to use.

It seems obvious to me that the when clause should take effect only after it's defined. And have the scope of the current block.

ben.davies...@gmail.com

unread,
Mar 8, 2016, 5:09:33 AM3/8/16
to golang-nuts, mikael....@gmail.com
See, to me that just looks misleading; if I read that quickly it looks like the success path would result in the return readFromDB() line.

But I actually prefer the explicitness of "if err != nil {}" 

Mikaël Cluseau

unread,
Mar 8, 2016, 5:15:15 AM3/8/16
to ben.davies...@gmail.com, golang-nuts
On 03/08/2016 09:09 PM, ben.davies...@gmail.com wrote:
> See, to me that just looks misleading; if I read that quickly it looks
> like the success path would result in the return readFromDB() line.

All but the the first line you mean? ;-)

> But I actually prefer the explicitness of "if err != nil {}"

The code was actually a lot of if err == nil. Not the best case for
factorizing.

Mikaël Cluseau

unread,
Mar 8, 2016, 3:16:47 PM3/8/16
to ben.davies...@gmail.com, golang-nuts
On 03/08/2016 09:14 PM, Mikaël Cluseau wrote:
On 03/08/2016 09:09 PM, ben.davies...@gmail.com wrote:
See, to me that just looks misleading; if I read that quickly it looks like the success path would result in the return readFromDB() line.

All but the the first line you mean? ;-)

Honestly (still thinking about it) "reading quickly" the original code triggers the same feeling to me. You have many if statements mixing "if err != nil" and "if err == nil", and I would say it even more confusing; in fact, I'd even push it to say that at least with "when" the way is cleaner, only the error cases are left. To further address your issue, what if I rewrite the last line:


func readData() (data X, err error) {
   
when err == nil {
        return data, nil
   
}

    data
, err = readFromRedis()
   
if err.(RedisError) != redis.NotFoundError {
       
return nil, err
   
}

    data
, err = readDataFromService()


   
if er, ok := err.(ServiceError); ok && er != service.NotAvailableError &&

        err.(ServiceError) != service.NotFoundError {
       
return nil, err
   
}


    data, err :=
readFromDB()
    return nil, err
}

We can also push the "not found" test away and normalize it, and get even farther:

func isNotFound(err error) bool {
    // one could wish to return err == redis.NFE || err == service.NAE || err == service.NFE
    // but lets stay close to the original code.
    switch err.(type) {
    case RedisError:
        return err == redis.NotFoundError
    case ServiceError:
        return err ==
service.NotAvailableError || err == service.NotFoundError
    default:
        return false
    }
}

func readData() (data X, err error) {
   
when err == nil {
        return data, nil
   
} else if !isNotFound(err) {
       
return nil, err
    }
    data
, err = readFromRedis()
    data, err = readDataFromService()
    data, err = readFromDB()
    return nil, err
}

How does that feel compared to the following code?


func readData() (data X, err error) {
    data, err = readFromRedis()
    if
err == nil {
        return data, nil
   
} else if !isNotFound(err) {
       
return nil, err
    }

    data, err = readDataFromService()
    if
err == nil {
        return data, nil
   
} else if !isNotFound(err) {
       
return nil, err
    }

    data, err = readFromDB()
    if
err == nil {
        return data, nil
   
} else if !isNotFound(err) {
       
return nil, err
    }
    return nil, err
}

Or even compared to the following form, quite obvious given the when form?

func readData() (data X, err error)
{
    for
readFunc in range []func() (data, error){
        readFromRedis,
        readDataFromService,
       
readFromDB,
    } {
        data, err = readFunc()
        if
err == nil {
            return data, nil
       
} else if !isNotFound(err) {
           
return nil, err
        }
    }

    return nil, err
}

Reply all
Reply to author
Forward
0 new messages