This is the actual code that caused me to write the ticket above (be warned, I wouldn't consider it amazing code; my first foray into writing a web app as a side project, just trying to get something that works):
Basically, I have several pages I'm rendering, which have common prerequisites regarding checks, and common handling processes (passing some sanitized data to a template). The *GetDisplay() functions take a structure from the "database" layer and sanitize it / process it for handing to the templates. The two *GetDisplay() functions return pointers to two different types, appropriate for the template to which they will be passed; and return nil if there's an issue.
So I have a map, `data` of type `map[string]interface{}` that I pass into the templates; and two different paths set `data["Display"]`; then at the end I want to check if either of the `*GetDisplay()` functions returned `nil`. So naturally, the first version of the code checked `data["Display"] == nil`, which was always false, since it was implicitly checking `data["Display"] == interface{}(nil)`, but the value in case of an error would be either `*DiscussionDisplay(nil)` or `*UserDisplay(nil)`.
I mean, sure, there are other ways to structure this; I could return an error or a boolean in addition to returning nil. But 1) the only reason to do that is to work around this language limitation 2) it's a "foot gun" that it's easy to fall into.
And sure, a golang developer who'd shot themselves in the foot a few times with this would catch it during review; but I don't think a bunch of newer developers would catch it, even if they had extensive experience in other languages.
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.
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.
https://github.com/gwd/session-scheduler/blob/master/handle_...
Basically, I have several pages I'm rendering, which have common prerequisites regarding checks, and common handling processes (passing some sanitized data to a template). The *GetDisplay() functions take a structure from the "database" layer and sanitize it / process it for handing to the templates. The two *GetDisplay() functions return pointers to two different types, appropriate for the template to which they will be passed; and return nil if there's an issue.
So I have a map, `data` of type `map[string]interface{}` that I pass into the templates; and two different paths set `data["Display"]`; then at the end I want to check if either of the `*GetDisplay()` functions returned `nil`. So naturally, the first version of the code checked `data["Display"] == nil`, which was always false, since it was implicitly checking `data["Display"] == interface{}(nil)`, but the value in case of an error would be either `*DiscussionDisplay(nil)` or `*UserDisplay(nil)`.
I mean, sure, there are other ways to structure this; I could return an error or a boolean in addition to returning nil. But 1) the only reason to do that is to work around this language limitation 2) it's a "foot gun" that it's easy to fall into.
And sure, a golang developer who'd shot themselves in the foot a few times with this would catch it during review; but I don't think a bunch of newer developers would catch it, even if they had extensive experience in other languages.