4
\$\begingroup\$

The MediaWiki API may return multiple errors. Is this a idiomatic way of representing them?

type MediaWikiError struct {
    Code    string      `json:"code"`
    Data    interface{} `json:"data"`
    Message string      `json:"text"`
    Module  string      `json:"module"`
}

func (e *MediaWikiError) Error() string {
    return fmt.Sprintf("MediaWiki module \"%s\" reported an error: %s (%s)", e.Module, e.Message, e.Code)
}

type MediaWikiErrors []*MediaWikiError

func (e MediaWikiErrors) Error() string {
    switch len(e) {
    case 0:
        return ""
    case 1:
        return e[0].Error()
    default:
        codes := make([]string, len(e))
        for i, err := range e {
            codes[i] = err.Code
        }
        return fmt.Sprintf("MediaWiki API reported multiple errors: %s", strings.Join(codes, ", "))
    }
} 
\$\endgroup\$
1
+100
\$\begingroup\$

1. Don't use \" in string

Go provide a very nice way to handle quote (") in strings:

instead of

fmt.Sprintf("MediaWiki module \"%s\" reported an error: %s (%s)", ...)

you can use

fmt.Sprintf(`MediaWiki module "%s" reported an error: %s (%s)`, ...)

It's easier and nicer to read

2. Use range loop instead of switch

Instead of specifiying behavior for 3 cases ( 0, 1, default), we can use a simple range loop

So this

func (e MediaWikiErrors) Error() string {
    switch len(e) {
    case 0:
        return ""
    case 1:
        return e[0].Error()
    default:
        codes := make([]string, len(e))
        for i, err := range e {
            codes[i] = err.Code
        }
        return fmt.Sprintf("MediaWiki API reported multiple errors: %s", strings.Join(codes, ", "))
    }
} 

become simply this:

func (e MediaWikiErrors) Error() (errMsg string) {
    for _, err := range e {
        errMsg += err.Error() + "\n"
    }
    return errMsg
} 

I prefer joining errors with a line return (\n) instead of a comma, but it's just my personal taste. Multiple errors would be printed like this:

MediaWiki module "module_1" reported an error: parsing error (3)
MediaWiki module "module_2" reported an error: access denied (22)
\$\endgroup\$
5
  • \$\begingroup\$ That are some useful suggestion, but it doesn't answer my question. Is it idiomatic to represent multiple errors in this way? \$\endgroup\$ – user103180 May 23 '18 at 5:43
  • \$\begingroup\$ @R3turnz I guess it is. The MongoDB driver use something similar to handle bulk insert error for example. The best way to now is to look at how the standard library / project maintained by go authors handle similar problems \$\endgroup\$ – felix May 23 '18 at 6:07
  • \$\begingroup\$ Do you have an example where the Go authors faced a similar problem? \$\endgroup\$ – user103180 May 23 '18 at 6:37
  • \$\begingroup\$ @R3turnz maybe golint source. It reads .go file, and then print all "style" error \$\endgroup\$ – felix May 23 '18 at 7:10
  • \$\begingroup\$ In addition to your comment wrt using back-tick string literals you can also get rid of the quotes in the format string by replacing "%s" with %q which is more readable and handles issues such as as the string argument itself contains a character that needs quoting. \$\endgroup\$ – Dave C Sep 3 '18 at 19:33
0
\$\begingroup\$

Yes, it is idiomatic. You have access to all relevant details of the individual errors, as well as you have implemented the error interface.

I don't think there should ever be an empty MediawikiErrors object, or at least its Error function should never be called, but for convenience returning an empty string there is fine.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy