Is casting a concrete type to an interface and using it if it successful bad practice?

Question

I recently came across some code:

val, ok := i.(SomeInterface)
if ok {
    val.Method()
}

The above is Go, and attempts to cast to an interface and then runs the method for that interface against the struct if it is, if not it doesn't bother. I'm not really fussed on the specifics of how Go works, but the more general idea.

I've not really seen this idea in practice, and the idea is simple but powerful when working with generics. I have a gut feeling that this is bad, but having it available is really handy.

My specific use case is that I'm writing some firestore parsing code, which takes a doc and creates a object out of it. Firestore has a helpful 'dataTo' method that will parse it to the struct doing your typical field matching etc. BUT for some structs I need the ref to the doc, which I want something like object.Ref = document.Ref. My choices seem limited:

• I accept the bloat and make all structs implement the interface to set the ID and make my generic function take that type, but then have a bunch of structs with useless stuff.
• Provide two functions, one that handles this case and one that does not, and support both functions, which is a bit of duplication.

Faced with the above, this little statement seems nice and neat, and honestly safe. I can check if it implements a 'SetRef' interface and if it does, use it and set the ref on the struct.

I have done some research and come across upcasting and downcasting in the OO world, and I believe this is very similar to upcasting, but I'm not sure if it strictly is, as I'm sort of asking 'is it this superclass' rather than 'I know it's this superclass, I'll cast to it' which seems like an important distinction.

Am I seeing ghosts in the closet here?

Answer 1

Generally it is frowned upon. You'll find more information about this in threads asking if if (X is T) { stuff(); } style code is acceptable.

If you're using it, that means that you have a collection with mixed types (uniform types would either all have the interface or not). Making decisions based on type markers tends to be slow and error prone and fragile across serialization boundaries. It's better to make decisions off of data, where possible.

Golang makes this trickier with their lack of generic collections. I don't know the language well enough to know if your snippet is idiomatic there. It might be.

And this is inoffensive enough that I wouldn't spend too much time on it. If there's an obvious way to avoid the runtime typecheck, prefer that. If not, focus on getting stuff done.

Answer 2

You can have a situation where multiple classes may or may not implement an interface, and the caller can call the interface if implemented, and know what to do when the interface doesn't exist.

For example take an interface with two methods getColor and setColor. If the interface is implemented then you enable the "color" menu and call getColor to display the correct color, and setColor to set the color the user chooses. If the interface is not implemented then you disable the "color" menu (and probably make you sure you display everything in some standard color). So the fact that it is not implemented tells the caller what they need to know. That is an absolutely fine pattern. Especially if you have more obscure interfaces where many objects really dont care about implementing them.

I hope you notice that the decision was not "if interface implemented then call it else don't call it". If you do that blindly then that would be very bad. In my example, the existence / non existence of the interface required quite a bit of code to handle the situation. Now think what you would do if you wanted to save the color of an item in a document. You'd have to call "getColor" and do lots of other appropriate code if implemented, and do nothing at all if it is not implemented.

Answer 3

It's all a question of balance:

• testing for the type, and conditionally calling a type-specific method is not polymorphic nor a generic design and "tell don't ask" should be preferred
• However, if the method in question does not really belong to the interface, "tell don't ask" might work against the single responsibility principle, and you'd need some kind of adapter as you described. This can make sense if there are a few adapters, but might also lead to a lot of fluffy adapter code.
• Therefore, the conditional approach, even if it looks primitive, can perfectly make sense, if the conditional call is rather exceptional.

Answer 4

This approach is something an OO purist would probably tell you indicates an issue with your design or the design of the libraries you are working with, perhaps. There's something to this but OO pragmatism has largely defeated OO fundamentalism over the years and whether this is a code smell depends on a lot of things. It's hard to know without looking at the larger structure of the design but I get the sense from your description that this is likely necessary or avoiding it might not be worth the trouble.

I think the reason this kind of approach is "frowned upon" as Telastyn correctly notes, is because it's associated with a common fundamental misunderstanding of OO. In a nutshell, many people have been taught that the way you design classes is to think about your entities in terms of their properties and capabilities and then you define interfaces for things that these classes share. That approach tends to lead to these kinds of runtime checks due to the fact that your classes will necessarily be different (i.e. why would you need more than one if they weren't?)

For example, a dog can walk and run, and a bird can walk and fly. If you have a function that, say, takes a collection of things and have them attempt to flee, you don't want to call the common behavior (walk). You need to call a different behavior on dogs than on birds and you need to know which one you are dealing with in order to do that.

Instead, I argue that the better way to design interfaces is instead to derive them from what function (or methods, the distinction is unimportant) are going to do with them. Taking the example above, the function is about calling the animals 'flee' behavior. An interface designed to be used with this method would therefore have a flee method and any object that is to be a passed to it needs to define what that to do when told to flee. In other words, the interface tells you what behaviors the arguments to a function must support.

I think this is a somewhat subtle (and perhaps unintuitive) distinction, but it makes a big difference. Interfaces are not there to tell you how classes are similar. They tell you what capabilities are required of arguments. For example, consider something like a Comparable interface. The point of implementing that is so that you can pass objects to pre-built sorting routines that know nothing else about the objects passed to it. The pattern you describe simply fails to support that kind of reusable capability because it needs to 'know' all the kinds of objects that it will handle when it is written.

As I mentioned before, it's possible that your approach is the most sensible option here so don't twist your code in knots trying to avoid it. Another thing to be aware of is that closure-style or anonymous classes are a great way to present an object to function with the required interface without having to implement it as part of the top-level class definition.