59

Assuming a language with some inherent type safety (e.g., not JavaScript):

Given a method that accepts a SuperType, we know that in most cases wherein we might be tempted to perform type testing to pick an action:

public void DoSomethingTo(SuperType o) {
  if (o isa SubTypeA) {
    o.doSomethingA()
  } else {
    o.doSomethingB();
  }
}

We should usually, if not always, create a single, overridable method on the SuperType and do this:

public void DoSomethingTo(SuperType o) {
  o.doSomething();
}

... wherein each subtype is given its own doSomething() implementation. The rest of our application can then be appropriately ignorant of whether any given SuperType is really a SubTypeA or a SubTypeB.

Wonderful.

But, we're still given is a-like operations in most, if not all, type-safe languages. And that suggests a potential need for explicit type testing.

So, in what situations, if any, should we or must we perform explicit type testing?

Forgive my absent mindedness or lack of creativity. I know I've done it before; but, it was honestly so long ago I can't remember if what I did was good! And in recent memory, I don't think I've encountered a need to test types outside my cowboy JavaScript.

svidgen
  • 13,414
  • 2
  • 34
  • 60
  • 1
    Read about [type inference](http://en.wikipedia.org/wiki/Type_inference) and [type systems](http://en.wikipedia.org/wiki/Type_system) – Basile Starynkevitch Aug 18 '14 at 19:10
  • 4
    It's worth pointing out that neither Java nor C# had generics in their first version. You had to cast to and from Object to use containers. – Doval Aug 18 '14 at 19:11
  • 6
    "Type checking" almost always refers to checking whether code observes the static typing rules of the language. What you mean is usually called type *testing*. –  Aug 18 '14 at 19:22
  • @delnan Not sure "checking" is an invalid way to phrase it; but, I'll change it for clarity. – svidgen Aug 18 '14 at 19:23
  • 1) If the type you're checking for is an interface 2) If there is a limited amount of subtypes. – CodesInChaos Aug 18 '14 at 19:24
  • @CodesInChaos: IMHO these things can give some evidence that using a "is-a" operator *might* be the right tool, but I think they don't indicate that we "should or must perform explicit type testing" (as the OP wrote). – Doc Brown Aug 18 '14 at 20:51
  • 2
    See also: [When to use run-time type information?](http://stackoverflow.com/q/1520466) – Caleb Aug 18 '14 at 21:00
  • 3
    This is why I like writing C++ and leaving RTTI turned off. When one literally cannot test object types at runtime, it forces the developer to adhere to good OO design with regards to the question being asked here. –  Aug 18 '14 at 21:43
  • One Java example is testing if `someList instanceof RandomAccess`. – wchargin Aug 19 '14 at 23:11
  • 1
    Stop breaking parametricity :) – Ven Aug 20 '14 at 12:33
  • 1
    Also if the two method calls expect wildly different arguments from the caller `o.doSomethingA(int,int,String,float) and o.doSomethingB(bool,int,double,Object)` it would be crazy to let both implement a polymorphic method which accepts all 8 parameters and only uses half of them in each implementation... – Falco Aug 20 '14 at 14:48

14 Answers14

55

"Never" is the canonical answer to "when is type testing okay?" There's no way to prove or disprove this; it is part of a system of beliefs about what makes "good design" or "good object-oriented design." It's also hokum.

To be sure, if you have an integrated set of classes and also more than one or two functions that need that kind of direct type testing, you're probably DOING IT WRONG. What you really need is a method that's implemented differently in SuperType and its subtypes. This is part and parcel of object-oriented programming, and the whole reason classes and inheritance exist.

In this case, explicitly type-testing is wrong not because type testing is inherently wrong, but because the language already has a clean, extensible, idiomatic way of accomplishing type discrimination, and you didn't use it. Instead, you fell back to a primitive, fragile, non-extensible idiom.

Solution: Use the idiom. As you suggested, add a method to each of the classes, then let standard inheritance and method-selection algorithms determine which case applies. Or if you can't change the base types, subclass and add your method there.

So much for the conventional wisdom, and on to some answers. Some cases where explicit type testing makes sense:

  1. It's a one-off. If you had a lot of type discrimination to do, you might extend the types, or subclass. But you don't. You have just one or two places where you need explicit testing, so it's not worth your while to go back and work through the class hierarchy to add the functions as methods. Or it's not worth the practical effort to add the kind of generality, testing, design reviews, documentation, or other attributes of the base classes for such a simple, limited usage. In that case, adding a function that does direct testing is rational.

  2. You can't adjust the classes. You think about subclassing--but you can't. Many classes in Java, for instance, are designated final. You try to throw in a public class ExtendedSubTypeA extends SubTypeA {...} and the compiler tells you, in no uncertain terms, that what you're doing is not possible. Sorry, grace and sophistication of the object oriented model! Someone decided you can't extend their types! Unfortunately, many of the standard library are final, and making classes final is common design guidance. A function end-run is what's left available to you.

    BTW, this isn't limited to statically typed languages. Dynamic language Python has a number of base classes that, under the covers implemented in C, cannot really be modified. Like Java, that includes most of the standard types.

  3. Your code is external. You are developing with classes and objects that come from a range of database servers, middleware engines, and other codebases you can't control or adjust. Your code is just a lowly consumer of objects generated elsewhere. Even if you could subclass SuperType, you're not going to be able to get those libraries on which you depend to generate objects in your subclasses. They're going to hand you instances of the types they know, not your variants. This isn't always the case...sometimes they are built for flexibility, and they dynamically instantiate instances of classes that you feed them. Or they provide a mechanism to register the subclasses you want their factories to construct. XML parsers seem particularly good at providing such entry points; see e.g. a JAXB example in Java or lxml in Python. But most code bases do not provide such extensions. They're going to hand you back the classes they were built with and know about. It generally will not make sense to proxy their results into your custom results just so you can use a purely object-oriented type selector. If you're going to do type discrimination, you're going to have to do it relatively crudely. Your type testing code then looks quite appropriate.

  4. Poor person's generics/multiple dispatch. You want to accept a variety of different types to your code, and feel that having an array of very type-specific methods isn't graceful. public void add(Object x) seems logical, but not an array of addByte, addShort, addInt, addLong, addFloat, addDouble, addBoolean, addChar, and addString variants (to name a few). Having functions or methods that take a high super-type and then determine what to do on a type-by-type basis--they're not going to win you the Purity Award at the annual Booch-Liskov Design Symposium, but dropping the Hungarian naming will give you a simpler API. In a sense, your is-a or is-instance-of testing is simulating generic or multi-dispatch in a language context that doesn't natively support it.

    Built-in language support for both generics and duck typing reduce the need for type checking by making "do something graceful and appropriate" more likely. The multiple dispatch / interface selection seen in languages like Julia and Go similarly replace direct type testing with built-in mechanisms for type-based selection of "what to do." But not all languages support these. Java e.g. is generally single-dispatch, and its idioms are not super-friendly to duck typing.

    But even with all these type discrimination features--inheritance, generics, duck typing, and multiple-dispatch--it's sometimes just convenient to have a single, consolidated routine that makes the fact that you are doing something based on the type of the object clear and immediate. In metaprogramming I have found it essentially unavoidable. Whether falling back to direct type inquires constitutes "pragmatism in action" or "dirty coding" will depend on your design philosophy and beliefs.

Jonathan Eunice
  • 9,710
  • 1
  • 31
  • 42
  • If an operation cannot be performed upon a particular type, but can be performed upon two different types to which it would be implicitly convertible, overloading is only appropriate if both conversions would yield identical results. Otherwise one ends up with crummy behaviors like in .NET: `(1.0).Equals(1.0f)` yielding true [argument promotes to `double`], but `(1.0f).Equals(1.0)` yielding false [argument promotes to `object`]; in Java, `Math.round(123456789*1.0)` yields 123456789, but `Math.round(123456789*1.0)` yields 123456792 [argument promotes to `float` rather than `double`]. – supercat Aug 18 '14 at 22:55
  • This is the classic argument against automatic type casting/escalation. Bad and paradoxical results, at least in edge cases. I agree, but not sure how you intend it to relate to my answer. – Jonathan Eunice Aug 18 '14 at 23:09
  • I was responding to your point #4 which seemed to be advocating overloading rather than using differently-named methods with different types. – supercat Aug 18 '14 at 23:13
  • Point taken. Languages like Perl, PHP, and Ruby have very eager automatic type conversion; they're successful and practical, but I wouldn't say they have the most elegant type systems. The problems you site are big reason you see so much `===` in PHP code; the original `==` much less useful in an environment that auto-fiddles the operands. But different language communities have very different philosophies. Java and Microsoft-centric more likely to go Hungarian style; Python and dynamic languages prefer duck typing / "overloaded" methods. – Jonathan Eunice Aug 18 '14 at 23:36
  • 2
    @supercat Blame my poor eyesight, but the two expressions with `Math.round` look identical to me. What's the difference? – Lily Chung Aug 19 '14 at 17:29
  • 2
    @IstvanChung: Ooops... the latter should have been `Math.round(123456789)` [indicative of what might happen if someone rewrites `Math.round(thing.getPosition() * COUNTS_PER_MIL)` to return an unscaled position value, not realizing that `getPosition` returns an `int` or `long`.] – supercat Aug 19 '14 at 17:46
  • I disagree with never. I think you could put forward a solid argument for "only when making non-functional changes". For example, you might look at the type of a Collection to see if it supports random access if that makes your algorithm faster, otherwise you still work, you just do it slower. Obviously this is something you'd do in the last stages of a project, not while you were still working out correctness. – Phoshi Aug 20 '14 at 12:42
27

The main situation I've ever needed it was when comparing two objects, such as in an equals(other) method, which might require different algorithms depending on the exact type of other. Even then, it's fairly rare.

The other situation I've had it come up, again very rarely, is after deserialization or parsing, where you sometimes need it to safely cast to a more specific type.

Also, sometimes you just need a hack to work around third party code you don't control. It's one of those things you don't really want to use on a regular basis, but are glad it's there when you really need it.

Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
  • 1
    I was momentarily delighted by the deserialization case, having falsely remembered using it there; but, now I can't imagine how I would have! I know I've done some weird type-lookups for that; but I'm not sure whether that constitutes type *testing*. Maybe serializing is a closer parallel: having to interrogate objects for their concrete type. – svidgen Aug 18 '14 at 19:34
  • 1
    Serializing is usually doable using polymorphism. When deserializing, you often do something like `BaseClass base = deserialize(input)`, because you don't know the type yet, then you do `if (base instanceof Derived) derived = (Derived)base` to store it as its exact derived type. – Karl Bielefeldt Aug 18 '14 at 19:42
  • 2
    Equality makes sense. In my experience, such methods often have a form like “if these two objects have the same concrete type, return whether all their fields are equal; otherwise, return false (or noncomparable)”. – Jon Purdy Aug 18 '14 at 20:07
  • 2
    In particular, the fact that you find yourself testing type is a good indicator that polymorphic equality comparisons are a nest of vipers. – Steve Jessop Aug 18 '14 at 23:36
  • @KarlBielefeldt Are you saying that it's rate to need to compare two objects or that it is rare that such comparison would require type testing? – Mehdi Charife Aug 27 '23 at 16:07
12

The standard (but hopefully rare) case looks like this: if in the following situation

public void DoSomethingTo(SuperType o) {
  if (o isa SubTypeA) {
    DoSomethingA((SubTypeA) o )
  } else {
    DoSomethingB((SubTypeB) o );
  }
}

the functions DoSomethingA or DoSomethingB cannot easily be implemented as member functions of the inheritance tree of SuperType / SubTypeA/ SubTypeB. For example, if

  • the subtypes are part of a library which you cannot change, or
  • if adding the code for DoSomethingXXX to that library would mean to introduce a forbidden dependency.

Note there are often situations where you can circumvent this problem (for example, by creating a wrapper or adapter for SubTypeA and SubTypeB, or trying to re-implement DoSomething completly in terms of basic operations of SuperType), but sometimes these solutions are not worth the hassle or make things more complicated and less extensible than doing the explicit type test.

An example from my yesterdays work: I had a situation where I was going to parallelize the processing of a list of objects (of type SuperType, with exactly two different subtypes, where it is extremely unlikely that there will ever be more). The unparallelized version contained two loops: one loop for objects of subtype A, calling DoSomethingA, and a second loop for objects of subtype B, calling DoSomethingB.

The "DoSomethingA" and "DoSomethingB" methods are both time intensive calculations, using context information which is not available at the scope of the subtypes A and B. (so it makes no sense to implement them as member functions of the subtypes). From the viewpoint of the new "parallel loop", it makes things a lot easier by dealing with them uniformly, so I implemented a function similar to DoSomethingTo from above. However, looking into the implementations of "DoSomethingA" and "DoSomethingB" shows they work very differently internally. So trying to implement a generic "DoSomething" by extending SuperType with a lot of abstract methods was not really going to work, or would mean to overdesign things completely.

Doc Brown
  • 199,015
  • 33
  • 367
  • 565
  • 2
    Any chance you can add a small, concrete example to convince my foggy brain this scenario isn't contrived? – svidgen Aug 18 '14 at 19:27
  • To be clear, I'm not saying it *is* contrived. I suspect I'm just supremely fog-brained today. – svidgen Aug 18 '14 at 19:36
  • @svidgen: this is far from beeing contrived, actually I encountered these situation today (though I cannot post this example here because it contains business internals). And I agree to the other answers that using the "is" operator should be an exception and only done in rare cases. – Doc Brown Aug 18 '14 at 19:37
  • I think your edit, which I overlooked, gives a good example. ... Are there instances wherein this is OK even when you *do* control `SuperType` and it's subclasses? – svidgen Aug 18 '14 at 19:40
  • 6
    Here's a concrete example: the top-level container in a JSON response from a web service can be either a dictionary or an array. Typically, you have some tool that turns the JSON into real objects (e.g. `NSJSONSerialization` in Obj-C), but you don't want to simply trust that the response contains the type that you expect, so before using it you check it (e.g. `if ([theResponse isKindOfClass:[NSArray class]])...`). – Caleb Aug 18 '14 at 19:43
  • @svidgen: even if you have full control over `Supertype`, implementing `DoSomething` in a generic manner might enforce you to introduce more than a dozen new overridable methods into `Supertype`. Sometimes that's the right thing to do, but in a situation where you have just two sub types and it is unlikely there will be more subtypes within the next years, these maybe a "design overkill", compared to a simple "isa" test. – Doc Brown Aug 18 '14 at 19:49
  • @Caleb That's a job for an [algebraic data type](http://en.wikipedia.org/wiki/Algebraic_data_type). As I mentioned in [another comment](http://programmers.stackexchange.com/questions/253704/when-is-type-testing-ok#comment510470_253707), mainstream OOP languages don't support them, so when you need them you end up *faking them* with inheritance. The big giveaway is the the fact that the number of "subtypes" is fixed - in your JSON example, there's no reason to ever add new classes of JSON values unless the standard changes (i.e. the code no longer models reality). – Doval Aug 19 '14 at 11:26
  • As an even simpler example: determining the number of items in an `IEnumerable`. – supercat Aug 19 '14 at 18:57
5

As Uncle Bob calls it:

When your compiler forgets about the type.

In one of his Clean Coder episodes he gave an example of a function call that is used to return Employees. Manager is a sub-type of Employee. Let's assume we have an application service that accepts a Manager's id and summons him to office :) The function getEmployeeById() return a super-type Employee , but I want to check if a manager is returned in this use-case.

For example:

var manager = employeeRepository.getEmployeeById(empId);
if (!(manager is Manager))
   throw new Exception("Invalid Id specified.");
manager.summon();

Here I'm checking to see if the employee returned by query is actually a manager (i.e. I expect it to be a manager and if otherwise fail fast).

Not the best example, but it's uncle Bob after all.

Update

I updated the example as much as I can remember from memory.

Songo
  • 6,548
  • 4
  • 48
  • 89
  • 2
    Why doesn't `Manager`'s implementation of `summon()` just throw the exception in this example? – svidgen Aug 19 '14 at 01:29
  • @svidgen maybe the `CEO` can summon `Manager`s. – user253751 Aug 19 '14 at 03:27
  • @svidgen, It would then not be as clear that employeeRepository.getEmployeeById(empId) is expected to return an manager – Ian Aug 19 '14 at 08:52
  • @Ian I don't see that as a problem. If the calling code is asking for an `Employee`, it should only care that it gets something that behaves like an `Employee`. If different subclasses of `Employee` have different permissions, responsibilities, etc., what makes type-testing a better option than a real permissions system? – svidgen Aug 19 '14 at 14:25
3

When is type checking OK?

Never.

  1. By having the behavior associated with that type in some other function, you're violating the Open Closed Principle, because you're free to modify the existing behavior of the type by changing the is clause, or (in some languages, or depending on the scenario) because you can't extend the type without modifying the internals of the function doing the is check.
  2. More importantly, is checks are a strong sign that you're violating the Liskov Substitution Principle. Anything that works with SuperType should be completely ignorant of what sub types there may be.
  3. You're implicitly associating some behavior with the type's name. This makes your code harder to maintain because these implicit contracts are spread throughout the code and not guaranteed to be universally and consistently enforced like actual class members are.

All that said, is checks can be less bad than other alternatives. Putting all common functionality into a base class is heavy-handed and often leads to worse problems. Using a single class that has a flag or enum for what "type" the instance is... is worse than horrible, since you're now spreading the type system circumvention to all consumers.

In short, you should always consider type checks to be a strong code smell. But as with all guidelines, there will be times when you're forced to choose between which guideline violation is the least offensive.

Telastyn
  • 108,850
  • 29
  • 239
  • 365
  • 3
    There is one minor caveat: [implementing algebraic data types](http://programmers.stackexchange.com/questions/159804/how-do-you-encode-algebraic-data-types-in-a-c-or-java-like-language) in languages that don't support them. However, the use of inheritance and typechecking is purely a hacky implementation detail there; the intent isn't to introduce subtypes, but to categorize values. I bring it up only because ADTs are useful and **never** is a very strong qualifier, but otherwise I fully agree; `instanceof` leaks implementation details and breaks abstraction. – Doval Aug 18 '14 at 19:37
  • 18
    "Never" is a word I don't really like in such a context, especially when it contradicts what you write below. – Doc Brown Aug 18 '14 at 19:40
  • 11
    If by "never" you actually mean "sometimes," then you're right. – Caleb Aug 18 '14 at 19:44
  • 1
    @DocBrown & Caleb - It's always a code smell, and sometimes we need to write stinky code. That doesn't make it okay. – Telastyn Aug 18 '14 at 19:48
  • 2
    *OK* means allowable, acceptable, etc., but not necessarily ideal or optimal. Contrast with *not OK*: if something is *not OK* then you shouldn't do it at all. As you indicate, the need to check the type of something can be an indication of deeper problems in the code, but there are times when it's the most expedient, least bad option, and in such situations it's obviously *OK* to use the tools at your disposal. (If it were easy to avoid them in all situations, they probably wouldn't be there in the first place.) The question boils down identifying those situations, and *never* doesn't help. – Caleb Aug 18 '14 at 20:03
  • How else would you model the concept of "All X's can use a general-purpose method Y to do some operation, but some X's include a special-purpose method Z that can do it *better*". Either all X's have to implement an interface that includes Z (even if some don't actually support that method), or else one has to test which objects do support it. Personally, from a conceptual standpoint, I prefer the former approach, but SOLID principles and .NET's lack of support for default interface implementations argue against it. – supercat Aug 18 '14 at 22:58
  • I'm tempted to give you a +1 here for "most controversial answer." (+5/-4 at the time of this comment) – svidgen Aug 19 '14 at 14:26
  • @Telastyn: How should one write a method to efficiently retrieve the last item from an `IEnumerable`, if not by testing the type? Some types allow ways of retrieving the last item which are orders of magnitude faster than enumeration; should one forgo such speedups in the name of "code purity"? – supercat Aug 19 '14 at 19:05
  • 1
    @supercat - By virtual dispatch in an ideal world. That rather is the entire point of subtyping - have some variable implementation of a common behavior. Doing type testing yourself is circumventing the built in tools (and optimizations) (and safety checks) that the language and compiler provide for you. – Telastyn Aug 19 '14 at 19:17
  • @Telastyn: Using virtual dispatch to answer such queries would require that every implementation of `IEnumerable` provide support for all such queries, whether or not they could do anything better than a static method which accessed them only as `IEnumerable`. Personally, I would tend to favor such a design, Interface-Segregation Principle notwithstanding, but for the fact that .NET would make such a thing really painful to implement. Is there any way to use virtual dispatch without either using run-time type checking or having "kitchen-sink" base-level interfaces? – supercat Aug 19 '14 at 19:27
  • 1
    @supercat: One could argue that if you're trying to get the "last" element of an `IEnumerable`, you're Doing It Wrong. The sequence might not even be finite. And you shouldn't care. If you do, then use a more specific type from the start. – cHao Aug 19 '14 at 19:29
  • @supercat - you can do trickery with `dynamic`, but you can't get a nice `.Last` implementation then. In .NET, you're likely stuck with hackery since there's no default virtual implementation that does the naive thing and since there's no dynamic dispatch on extension methods. – Telastyn Aug 19 '14 at 19:33
  • An excellent point @cHao. C++ has `RandomAccessIterator` and the such to differentiate the capabilities of collections, but then usually uses templates to fake out dynamic dispatch on the capabilities. – Telastyn Aug 19 '14 at 19:34
  • @cHao: As a general principle, methods should demand the least specific type possible; given that collections whose last item can be found efficiently have no common ancestral type, what type could a method demand, other than `IEnumerable`, without needlessly restricting the circumstances where it may be employed? – supercat Aug 19 '14 at 19:40
  • 2
    @supercat: A methods should demand the least specific type possible *that satisfies its requirements*. `IEnumerable` doesn't promise a "last" element exists. If your method needs such an element, it should be requiring a type that guarantees the existence of one. And that type's subtypes can then provide efficient implementations of the "Last" method. – cHao Aug 19 '14 at 19:42
  • @cHao: Is there any type to which it is possible to cast every `IEnumerable` instance that has a last item which can be found efficiently, that guarantees the existence of such a last item? I would posit that it's more important that a method accept every object instance that has a last item, than that its type guarantee the existence of such an item. – supercat Aug 19 '14 at 19:53
  • @supercat: How about `ICollection` or `IList`? – cHao Aug 19 '14 at 20:03
  • @cHao: A `List` would satisfy `IEnumerable`, and could efficiently return its last item, but would not satisfy `ICollection`. A iterator which yields four instances of `Cat` and exits would also satisfy `IEnumerable` and could also quickly find the last item, but it doesn't implement `ICollection` either. Incidentally, some instances of collection types that implement `IList` won't have a last item, so implementation of `IList` is neither necessary nor sufficient to guarantee that it will be possible to efficiently return the last item. – supercat Aug 19 '14 at 20:34
  • @supercat: `ICollection` defines a `Count` property, which means there must be a count -- ie: some knowable number of items. And `IList` is a subtype of `ICollection`, and `ICollection` is a subtype of `IEnumerable`. – cHao Aug 19 '14 at 20:43
  • @cHao: A `List` implements `ICollection`, and a `List` implements `ICollection`, but while both implement `IEnumerable`, neither implements `ICollection`. – supercat Aug 19 '14 at 20:49
  • 1
    @supercat: Neither directly implements `IEnumerable` either; they work due to the covariance of the type parameter. And that covariance extends through `IList` and `ICollection` all the way back to `IEnumerable`. I don't see it skipping a generation. – cHao Aug 19 '14 at 20:53
  • @supercat: Ahh...just noticed that the other interfaces don't say `out T`. :P However, `IReadOnlyCollection` and `IReadOnlyList`, though, do. (And both are probably a better fit anyway, since you're not modifying the list.) – cHao Aug 19 '14 at 21:22
  • @cHao: There are a lot of types which semantically could implement `IReadOnlyCollection`, but don't [including all legacy `IList` implementations that predate `IReadOnlyCollection`]. If a method would be usable with such types, why shouldn't it be? – supercat Aug 19 '14 at 21:55
  • 2
    @supercat: Because if you say you take an `IEnumerable`, and are not more specific, you're saying you can take *any* `IEnumerable`. That is clearly not the case. And the second someone starts getting fancy with your method and it gets into an infinite loop trying to find the last element of an infinite sequence, your code will be entirely to blame. – cHao Aug 19 '14 at 23:07
  • @cHao: The fact that a method takes a parameter of a particular type does not imply that it will accept all instances of that type and its derivatives. It is entirely right and proper for methods to specify that object instances must abide by other criteria as well. For example, if one has a `List` and passes its `Sort` an `IComparable`, it is necessary not only that that comparator return integer values when given pairs of strings, but also that it does so in such a fashion as to yield a consistent ranking, at least for the particular strings in the list. – supercat Aug 19 '14 at 23:25
  • Comments are not for extended discussion; this conversation has been [moved to chat](http://chat.stackexchange.com/rooms/16545/discussion-on-answer-by-telastyn-when-is-type-testing-ok). –  Aug 19 '14 at 23:55
2

If you got a large code base (over 100K lines of code) and are close to shipping or are working in a branch that will later have to be merge, and therefore there is a lot cost/risk to changing a lot of cope.

You then sometimes have the option of a large refractor of the system, or some simple localised “type testing”. This creates a technical debt that should be paid back as soon as possible, but often is not.

(It is impossible to come up with an example, as any code that is small enough to be used as an example, is also small enough for the better design to be clearly visible.)

Or in other words, when the aim is to be paid your wage rather than to get “up votes” for the cleanness of your design.


The other common case is UI code, when for example you show a different UI for some types of employees, but clearly you don’t want the UI concepts to escape into all your “domain” classes.

You can you use “type testing” to decide what version of the UI to show, or have some fancy lookup table that converts from “domain classes” to “UI classes”. The lookup table is just a way of hiding the “type testing” in one place.

(Database updating code can have the same issues as UI code, however you tend to only have one set of database updating code, but you can have lots of different screens that must adapt to the type of object being shown.)

Ian
  • 4,594
  • 18
  • 28
  • The Visitor Pattern is often a good way to solve your UI case. – Ian Goldby Aug 19 '14 at 12:14
  • @IanGoldby, agreed at times it can be, however you are still doing "type testing", just hidden a bit. – Ian Aug 19 '14 at 16:17
  • Hidden in the same sense that it's hidden when you call a regular virtual method? Or do you mean something else? The Visitor Pattern I've used has no conditional statements that depend on type. It's all done by the language. – Ian Goldby Aug 20 '14 at 10:31
  • @IanGoldby, I meant hidden in the sense that it can make the WPF or WinForms code harder to understand. I expect that for some web base UIs it would work very well. – Ian Aug 20 '14 at 10:38
2

The implementation of LINQ uses lots of type checking for possible performance optimizations, and then a fallback for IEnumerable .

The most obvious example is probably the ElementAt method (tiny excerpt of .NET 4.5 source):

public static TSource ElementAt<TSource>(this IEnumerable<TSource> source, int index) { 
    IList<TSource> list = source as IList<TSource>;

    if (list != null) return list[index];
    // ... and then an enumerator is created and MoveNext is called index times

But there are lots of places in the Enumerable class where a similar pattern is used.

So perhaps optimizing for performance for a commonly-used subtype is a valid use. I am not sure how this could have been designed better.

  • It could have been designed better by providing a convenient means by which interfaces can supply default implementations, and then having `IEnumerable` include many methods like those in `List`, along with a `Features` property indicating which methods can be expected to work well, slowly, or not at all, as well as various assumptions a consumer can safely make about the collection (e.g. is its size and/or its *existing* contents guaranteed never to change [a type might support `Add` while still guaranteeing that existing contents would be immutable]). – supercat Nov 10 '14 at 23:40
  • Except in scenarios where a type might need to hold one of two completely different things at different times and the requirements are clearly mutually exclusive (and thus using separate fields would be redundant), I generally consider a need for try-casting to be a sign that members which should have been part of a base interface, weren't. That's not to say that code which uses an interface that should have included some members but doesn't shouldn't use try-casting to work around the base interface's omission, but those writing base interfaces should minimize clients' need to try-cast. – supercat Nov 10 '14 at 23:44
1

There is an example that comes up often in games developments, specifically in collision detection, that is difficult to deal with without the use of some form of type testing.

Assume that all game objects derive from a common base class GameObject. Each object has a rigid body collision shape CollisionShape which may provide a common interface (to say query position, orientation, etc) but the actual collision shapes will all be concrete subclasses such as Sphere, Box, ConvexHull, etc. storing information specific to that type of geometric object (see here for a real example)

Now, to test for a collisions I need to write a function for each pair of collision shape types:

detectCollision(Sphere, Sphere)
detectCollision(Sphere, Box)
detectCollision(Sphere, ConvexHull)
detectCollision(Box, ConvexHull)
...

that contain the specific math needed to perform an intersection of those two geometric types.

At each 'tick' of my game loop I need to check pairs of objects for collisions. But I only have access to GameObjects and their corresponding CollisionShapes. Clearly I need to know concrete types to know which collision detection function to call. Not even double dispatch (which is logically no different to checking for the type anyway) can help here*.

In practice in this situation the physics engines I've seen (Bullet and Havok) rely on type testing of one form or another.

I'm not saying this is necessarily a good solution, it's just that it may be the best of a small number of possible solutions to this problem

* Technically it is possible to use double dispatch in a horrendous and complicated way that would require N(N+1)/2 combinations (where N is the number of shape types you have) and would only obfuscate what you're really doing which is simultaneously finding out the types of the two shapes so I don't consider this to be a realistic solution.

Martin
  • 19
  • 1
1

Sometimes you do not want to add a common method to all the classes because it really is not their responsibility to perform that specific task.

For example you want to draw some entities but do not want to add the drawing code directly to them (which makes sense). In languages not supporting multiple dispatch you might end up with the following code:

void DrawEntity(Entity entity) {
    if (entity instanceof Circle) {
        DrawCircle((Circle) entity));
    else if (entity instanceof Rectangle) {
        DrawRectangle((Rectangle) entity));
    } ...
}

This becomes problematic when this code appears at several places and you need to modify it everywhere when adding new Entity type. If that is the case then it can be avoided by using the Visitor pattern but sometimes it is better to just keep things simple and not overengineer it. Those are the situations when type testing is OK.

Honza Brabec
  • 602
  • 4
  • 11
0

Type testing and type casting are two very closely related concepts. So closely related that I feel confident in saying that you should never do a type test unless your intent is to type cast the object based on the result.

When you think about ideal Object-Oriented design, type testing (and casting) should never happen. But hopefully by now you've figured out that Object-Oriented programming is not ideal. Sometimes, especially with lower-level code, the code can't stay true to the ideal. This is the case with ArrayLists in Java; since they don't know at run-time what class is being stored in the array, they create Object[] arrays and statically cast them to the correct type.

It's been pointed out that a common need to type testing (and type casting) comes from the Equals method, which in most languages is supposed to take a plain Object. The implementation should have some detailed checks to make if the two objects are the same type, which requires being able to test what type they are.

Type testing also comes up frequently in reflection. Often you will have methods that return Object[] or some other generic array, and you want to pull out all the Foo objects for whatever reason. This is a perfectly legitimate use of type testing and casting.

In general, type testing is bad when it needlessly couples your code to how a specific implementation was written. This can easily lead to needing a specific test for each type or combination of types, such as if you want to find the intersection of lines, rectangles, and circles, and the intersection function has a different algorithm for each combination. Your goal is to put any details specific to one kind of object in the same place as that object, because that will make it easier to maintain and extend your code.

meustrus
  • 109
  • 3
  • 1
    `ArrayLists` don't know the class being stored at runtime because Java didn't have generics and when they were finally introduced Oracle opted for backwards-compatibility with generics-less code. `equals` has the same issue, and it's a questionable design decision anyways; equality comparisons don't make sense for every type. – Doval Aug 18 '14 at 21:38
  • 2
    Technically, Java collections do _not_ cast their contents to anything. The compiler injects typecasts at the location of each access: e.g. `String x = (String) myListOfStrings.get(0)` –  Aug 18 '14 at 21:55
  • The last time I looked at the Java source (which may have been 1.6 or 1.5) there was an explicit cast in the ArrayList source. It generates a (suppressed) compiler warning for good reason but is allowed anyway. I suppose you could say that because of how generics are implemented, it was always an `Object` until it was accessed anyway; generics in Java only provide implicit casting made safe by compiler rules. – meustrus Aug 20 '14 at 14:42
0

The only time I use is is in combination with reflection. But even then is dynamic checking mostly, not hard-coded to a specific class (or only hard-coded to special classes such as String or List).

By dynamic checking I mean:

boolean checkType(Type type, Object object) {
    if (object.isOfType(type)) {

    }
}

and not hard-coded

boolean checkIsManaer(Object object) {
    if (object instanceof Manager) {

    }
}
Random42
  • 10,370
  • 10
  • 48
  • 65
0

It's acceptable in a case where you have to make a decision that involves two types and this decision is encapsulated in an object outside of that type hierarchy. For instance, let's say you're scheduling which object gets processed next in a list of objects waiting for processing:

abstract class Vehicle
{
    abstract void Process();
}

class Car : Vehicle { ... }
class Boat : Vehicle { ... }
class Truck : Vehicle { ... }

Now let's say our business logic is literally "all cars get precedence over boats and trucks". Adding a Priority property to the class doesn't allow you to express this business logic cleanly because you'll end up with this:

abstract class Vehicle
{
    abstract void Process();
    abstract int Priority { get }
}

class Car : Vehicle { public Priority { get { return 1; } } ... }
class Boat : Vehicle { public Priority { get { return 2; } } ... }
class Truck : Vehicle { public Priority { get { return 2; } } ... }

The problem is that now to understand the priority ordering you have to look at all the subclasses, or in other words you've added coupling to the subclasses.

You should of course make the priorities into constants and put them into a class by themselves, which helps keep scheduling business logic together:

static class Priorities
{
    public const int CAR_PRIORITY = 1;
    public const int BOAT_PRIORITY = 2;
    public const int TRUCK_PRIORITY = 2;
}

However, in reality the scheduling algorithm is something that might change in the future and it might eventually depend on more than just type. For instance, it might say "trucks over a weight of 5000 kg get special priority over all other vehicles." That's why the scheduling algorithm belongs in its own class, and it's a good idea to inspect the type to determine which one should go first:

class VehicleScheduler : IScheduleVehicles
{
    public Vehicle WhichVehicleGoesFirst(Vehicle vehicle1, Vehicle vehicle2)
    {
        if(vehicle1 is Car) return vehicle1;
        if(vehicle2 is Car) return vehicle2;
        return vehicle1;
    }
}

This is the most straightforward way to implement the business logic and still the most flexible to future changes.

Scott Whitlock
  • 21,874
  • 5
  • 60
  • 88
  • I don't agree with your particular example, but would do agree with the principle of having a private reference that may hold different types with different meanings. What I'd suggest as a better example would be a field which can either hold `null`, a `String`, or a `String[]`. If 99% of of objects will need exactly one string, encapsulating every string in a separately-constructed `String[]` may add considerable storage overhead. Handling the single-string case using a direct reference to a `String` will require more code, but will save storage and may make things faster. – supercat Nov 10 '14 at 23:35
0

Type testing is a tool, use it wisely and it can be a powerful ally. Use it poorly and your code will start to smell.

In our software we received messages over the network in response to requests. All deserialized messages shared a common base class Message.

The classes themselves were very simple, just the payload as typed C# properties and routines for marshalling and unmarshalling them (In fact I generated most of the classes using t4 templates from XML description of the message format)

Code would be something like:

Message response = await PerformSomeRequest(requestParameter);

// Server (out of our control) would send one message as response, but 
// the actual message type is not known ahead of time (it depended on 
// the exact request and the state of the server etc.)
if (response is ErrorMessage)
{ 
    // Extract error message and pass along (for example using exceptions)
}
else if (response is StuffHappenedMessage)
{
    // Extract results
}
else if (response is AnotherThingHappenedMessage)
{
    // Extract another type of result
}
// Half a dozen other possible checks for messages

Granted, one could argue that the message architecture could be better designed but it was designed a long time ago and not for C# so it is what it is. Here type testing solved a real problem for us in a not too shabby way.

Worth noting is that C# 7.0 is getting pattern matching (which in many respects is type testing on steroids) it can't be all bad...

Isak Savo
  • 231
  • 1
  • 4
0

Take a generic JSON parser. The result of a successful parse is an array, a dictionary, a string, a number, a boolean, or a null value. It can be any of those. And the elements of an array or the values in a dictionary can again be any of those types. Since the data is provided from outside your program, you have to accept any result (that is you have to accept it without crashing; you can reject a result that isn't what you expect).

gnasher729
  • 42,090
  • 4
  • 59
  • 119
  • Well, some JSON deserializers will attempt to instantiate an object tree if your structure either has type info for "inference fields" in it. But yeah. I think this is the direction Karl B's answer was headed in. – svidgen Dec 20 '16 at 23:06