2

I have been repeatedly told that explicit type conversions are an indicator of bad code and they should be avoided.

Now in all honesty I have been annoyed a bit by these claims due to the nature of my work which seems to require a lot of primitive type conversion. So I want to know if there is a better way to structure my code, or if graphics are an exception to this "bad code" heuristic.

In graphics you work 50% with a continuous space and 50% with a discrete space, which translate into floats and ints respectively.

One of the most common examples would be, calculate a 3D position using a continuous function, and then truncate the 3 values of that position to index a multidimensional array.

How would I organize my code such as to not do any type conversion? floats are needed to compute the continuous function, but cannot be used directly as array indices, as the compiler will reject statements in the form array[float] even if the float represents an integer.

  • "One of the most common examples would be, calculate a 3D position using a continuous function, and then truncate the 3 values of that position to index a multidimensional array." It is? What are you doing where that is a common operation? – Nicol Bolas Dec 4 '18 at 20:11
  • 2
    "I have been repeatedly told that explicit type conversions are an indicator of bad code and they should be avoided." It's hard to evaluate the utility/veracity of such statements when you do not provide links to those saying them or any idea of the context those statements come from. For example, was this statement made in relation to casting up/down class hierarchies? Was this statement made in relation to casting to/from void*? Etc. There are many kinds of explicit conversions. Your question is predicated on something which we have no real idea of or foundation for. – Nicol Bolas Dec 4 '18 at 20:13
  • 3D voxelization of geometry; 3D reconstruction of 2D textures and subsequent UV mapping onto the original texture to store values; Conversion of float coordinates to grid cells for the cached version of perlin noise... There's a few – Makogan Dec 4 '18 at 20:15
  • 1
    Right, but we're not talking about what you're saying. You are claiming that others are saying that such conversions are a code smell. I want some evidence of such statements or the context in which those statements were made. For example, I personally have never heard the suggestion that explicit conversions of primitive types was considered a code smell in and of itself. So I want to know what the foundation for your question is. – Nicol Bolas Dec 4 '18 at 20:20
  • 2
    Now look at the context. Your SO question asked to do casting, not "primitive type conversion". Casting, as a general operation, is dangerous. Casting can involve converting pointers to T to pointers to U and the like. Converting between integers and floats is not. You asked about a general thing and got a general answer, when what you wanted was a specific thing. – Nicol Bolas Dec 4 '18 at 20:26
6

The thing you have to remember is that every float-to-int conversion and vice versa potentially loses information. On most implementations, int can store a larger integer number with full precision than float, while float's decimal values are chopped off when converting to an int and that float can store larger numbers than can fit into an int.

Now for your specific use case, that may be perfectly fine or more specifically is exactly what you want. But in general, one should not take such conversions lightly.

Consider just the example from your SO question, which doesn't even involve float conversion:

int size = vector.size(); // Throws an implicit conversion warning
int size = (int)vector.size(); // C like typecasting is discouraged and forbidden in many code standards
int size = static_cast<int>vector.size(); // This makes me want to gauge my eyes out (it's ugly)

Each of these cases has a subtle bug: if the size of the vector is greater than numeric_limits<int>::max() (typically 2^31 - 1), then you aren't getting the actual size. So whatever you're doing with size is not going to work.

Now, you can say that you won't have a vector that big. And that may genuinely be true... today. How many security holes/bugs have been opened up because an application scaled to the point where some value overflowed the expected type? And how many bugs exist that are out there, lurking, waiting to pounce once some arbitrary size is exceeded?

That's why you get a warning when you don't explicitly convert it. That's why C++ uses syntax that "makes me want to gauge my eyes out". It's because what you're doing may not be safe. So you should carefully consider whether you ought to be doing it.

And that's where we get to:

3D voxelization of geometry; 3D reconstruction of 2D textures and subsequent UV mapping onto the original texture to store values; Conversion of float coordinates to grid cells for the cached version of perlin noise...

Each of those things should be hidden behind some interface which internally does the required conversion. And not a simplistic convert_int function; I mean one that is specific to the task in question. If you're converting normalized [0, 1] floats into pixel coordinates for a texture of some size, you have a function to do exactly that. It would be given the coordinates and the size of the texture, and it would return integer coordinates.

The point of the advice is that your code should not be littered with such naked conversions.

3

You are mistaking casting and conversions.

Converting between types is perfectly fine. For example, truncating a float to an int, or converting an into to a float, or parsing an int from a string. You are effectively calling some kind of constructor that's performing this conversion, though some conversions may be implicit. It is not possible to avoid conversions. However, such conversions might exhibit undefined behaviour if the value cannot be represented in the target type.

Casting between types is an indication that you are subverting the type system of the language and is usually unsafe. There are some safe casts such as upcasts, const casts, or std::move(). And sometimes casts are unavoidable in a very dynamic program. Especially in C, you will often cast to and from void* to work around the lack of generics. However, a modern C++ program will require casts very rarely.

  • A reinterpret_cast<T>(value) reinterprets a bit pattern of some object as an object of a different type. For example, we might reinterpret a float pointer as an int pointer. That doesn't convert the pointed-to value properly, we just end up with an int that has the same bit pattern. Such casts therefore tend to depend on implementation-defined or undefined behaviour, and are inappropriate in most cases (possible counter-examples: zero-overhead serialization, or cross-language ABIs).

  • A dynamic_cast<T>(value) performs a safe downcast of a polymorphic object. The existence of such casts is sometimes necessary if the type system is unable to describe some type information, but usually indicates that a class hierarchy was misdesigned: a user of an interface shouldn't have to downcast an object to a specific type, but should be able to perform all necessary operations through that interface. A dynamic cast can also raise an exception, and may be comparatively slow. There are also design patterns available that can avoid the need of explicit casts, for example the virtual copy constructor idiom, or the visitor pattern.

In your graphics work, you are unlikely to encounter the need for these problematic casts.

  • Which is why I am talking about casting values from primitive types such as ints, floats and doubles. I agree it is unlikely I run into polymorphic casting, my issue however is with primitve castng. Especially since I cannot design primitives to be one way or another, and writing wrappers for primitives seems like a bad idea. – Makogan Dec 4 '18 at 21:11
  • 2
    @amon: "For example, we might reinterpret a float value as an int." No, you can't. reinterpret_cast won't actually do that; if you try to compile int i = 5; float f = reinterpret_cast<float>(i);, you get a compile error. It can cast a pointer to an int to a pointer to a float, but that's a different thing entirely. – Nicol Bolas Dec 4 '18 at 21:28

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.