Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

    data := map[string]interface{}{}
So this is the problem, basically. Go isn't a dynamically typed language, and doesn't really let you create an arbitrary map of keys to objects like e.g. Javascript or Python does. Any time you see `map[something]interface{}` that's a huge red flag that something is fucky. In your case you want to define `data` as a struct type with a Display field (and whatever else).

    if ... reflect.ValueOf(display).IsNil() {
Any use of `package reflect` in application code is a similarly huge red flag. 99 times out of 100 it's a design error that ought to be fixed.


> In your case you want to define `data` as a struct type with a Display field (and whatever else).

So first of all, the reason things are defined that way is to interact with the golang templating libraries. Secondly, your suggestion wouldn't really solve the issue in this case: content of "Display" is different for each web page, and so the only way to assign both types to the same value is to make it an interface.

> Any use of `package reflect` in application code is a similarly huge red flag. 99 times out of 100 it's a design error that ought to be fixed.

I'm not using reflect for fun; there is literally no other way to check for nil with interfaces (other than manually checking nil for all possible types).

At any rate, I wrote code in a way that's intuitive, at least to a C programmer (using 'nil' value as an indicator that there was an error); the code had a bug. Sure I could have rearchitected the whole function, and if this were a commercial product I may have. But it's a simple webapp to help scheduling discussions at my project's conferences; a quick fix that robustly works around Golang's deficiencies is perfectly reasonable.

EDIT: And honestly, there are exactly three ways of addressing this:

1. Separating the two page paths, duplicating all the logic which is common to the two. This makes it less DRY, which risks checks becoming inconsistent, increasing the chance that there will be a security issue.

2. Make the *GetDisplay() functions return a second value to indicate failure. This is honestly kind of a dumb thing to do to work around a language deficiency.

3. Continue to use 'nil' to indicate failure, and fix the check to be able to properly check for nil. This can be done by listing out the various possible values of 'nil', which is ugly, annoying, and fragile (since it would silently break if we added a third type); or it can be done using reflection.

#3 is obviously the most reasonable thing to do here.


    if display := data["Display"]; display == nil || reflect.ValueOf(display).IsNil() {
        <code>
I mean, even ignoring all of the interface{} design errors, the simple fix here is just

    if _, ok := data["Display"]; !ok {
        <code>
To reiterate, you should almost never need to interact with `interface{}` values in application code. If you find yourself trying to use, inspect, check, or otherwise program against `interface{}` values in application code, it almost always means that you're fighting the language, and that you need to change your approach to your problem.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: