Hacker News new | ask | show | jobs
by sltkr 735 days ago
Although using constexpr over #define wherever possible is good advice (and similarly with functions over macros), you can also argue that the culprit is really this snippet:

  float ts = 1 / FRAMERATE;
This blindly assumes that FRAMERATE is a floating-point constant, but there is no reason to assume it is; in theory, other parts of the code could depend on FRAMERATE being integral. The code should be written in a way that ensures conversion to float happens before division, for example:

  float ts = 1.0f / FRAMERATE;
5 comments

In a better language this mistake can't happen anyway. One of the things which too often gets missed when explaining why C++ is unsafe is that it's full of these unnecessary footguns, there is no need to be able to get this wrong.

C++ gets this wrong for maximum drop-in compatibility with C, the classic "New Jersey Style" programming language where simplicity of implementation is prized over simplicity of use or correctness.

> why C++ is unsafe is that it's full of these unnecessary footguns

I don't see the "footgun". C and C++ allow you to perform divisions by integers. If you don't specify the decimals, it understands that they are integers. It's how it's meant to be. In my opinion, calling that a "footgun" is like saying using single quotes for characters is a "footgun" because someone could interpret them as strings. That's just not understanding the language.

"just read the docs" is a fully general counter argument that can be used to justify arbitrary bad design decisions

I think Python 3 did the right thing by having 1/3 equal 0.333 (a float) rather than 0. It's more intuitive for the / operator to always do standard division and when you want integer division then you use the // operator instead. It's more consistent than having / return a completely different result depending on whether one of the operand happens to be 3 instead of 3.0

Who said that "standard division" is floating point division? Something which is inherently full of surprises, to the point that most programmers can't even explain the mechanism in detail?

> It's more consistent than having / return a completely different result depending on whether one of the operand happens to be 3 instead of 3.0

I'd argue that you should be able to learn a few relatively simple rules that are baked into the language, and which don't change. That's not much to ask considering the value you get out of it.

On the other end of the spectrum, you have lots of people vigorously defending their manually overloaded arithmetic operators, acting on their custom types. And to have class methods overloaded statically (depending on argument types) and dynamically (virtual dispatch). I'm assuming you're not part of that group?

Not saying that it isn't a footgun, though. My recommendation is to turn on warnings and you should be able to catch most of mistakes early.

Are you saying that it is intuitive that 4/2 should return a floating point number? Or the type of the result should depend on the value?

I would expect that no C or C++ programmer would find it intuitive

> I would expect that no C or C++ programmer would find it intuitive

My guess is that if instead C and C++ had two operators here you'd find C and C++ programmers fiercely defending this reality as the only correct choice.

The tribalism particularly in C++ is very strong. There are a handful of accepted topics for disparagement, such as the std::vector<bool> specialisation but outside that any questioning of orthodoxy is not well tolerated.

Aside from the flame-baity strawman, I'm not sure for which behaviour you are exactly arguing for.
> I don't see the "footgun".

    a / b
"Never" means "integer division" in regular math, so programming languages overloading it to sometimes mean integer division is a common suprise to newbies - especially when it's based on the types of `a` and `b`, which might not be determinable merely from reading the current file. Also, it's not equivalent to:

    a\b ≡ ⌊a/b⌋
As you might expect ( https://mathworld.wolfram.com/IntegerDivision.html ), but instead some round-to-zero nonsense that makes most uses of it on signed integers a bug IME. Some languages give integer division it's own syntax... others overload `a / b` to also mean path concatination.

And while it's not on my top 100 list of C or C++ footguns, it is one of those things that occasionally helps lead to a facepalm-inducing moment when even professionals have a brain fart and fail to consider it's overloaded behavior... or misremember the type of `a` or `b`... or change the type of `a` or `b`.

> That's just not understanding the language.

Learning the footguns and how to avoid them is an important part of understanding any language. That doesn't mean they aren't footguns, just that they can be (partially) ameliorated.

But the "better" languages have their own problems. UI calculations tend to need to mix int/float frequently, and the calculations can get fairly unreadable from all the casting (and I'm the sort that religiously casts to floats in C++ because I get burned on this). Swift, in particular, was particularly bad about this (Swift 5; I can't remember the details). Then there are things like `total / n` when calculating an average. Obviously you want `n` to be an int, because incrementing a float is a bad idea, but now you need to cast `n`, even though `total` is already a float, because they don't match.

I tend to prefer the casting, because I had spending a bunch of time debugging 0s and NaNs, but sometimes it makes things look unnecessarily ugly and hard to read.

> Then there are things like `total / n` when calculating an average. Obviously you want `n` to be an int, because incrementing a float is a bad idea, but now you need to cast `n`, even though `total` is already a float, because they don't match.

I don't know about other languages, but Python just lets you divide a float by an int, or vice versa, and it just always produces a float. Seems the most obvious thing to do.

I agree that implicitly promoting integers to floats (or implicitly changing any type) is a bad idea, but I don't think the C++ is uniquely bad in this regard. Java[1] and C#[2] both do this too.

[1] https://godbolt.org/z/vE9n14a54 [2] https://godbolt.org/z/aPa3P1Grc

Right, certainly I don't want to single C++ out for this - I mentioned it because (a) the article we're talking about is describing a mistake the author found in some C++ code, and (b) I wanted to head off the inevitable call to blame C, yes, C++ does it because C does it (and that's probably why Java and C# do it too) but that's a choice and it's a bad choice.

Implicit conversion is always a mistake. The price in terms of reduced clarity and extra mistakes is too high for the marginal convenience of less typing.

I feel the same for boolean coercion even, which I know is more controversial than some of the really stupid C promotions, I do not believe in "truthiness". There's only one false, it's the constant false, it's not 0 or "" or 0.0 or an empty array or a null pointer or a billion other things, it's just itself and nothing else.

Yeah I totally agree, it’s a bad design choice and the successors to C should not have copied it.
Drop-in compatibility with C is incredibly useful. I can't tell you how many times I went "man I just need to do X" and there was a C library floating around that did exactly what I wanted.

Most of C++ choices come with both positive and negative tradeoffs, despite what Rust users will say.

Define "better language". The duck typed languages all suffer the same problem.

Even in Go we had a stupid problem where default json deserializer creates floats (when deserialized into any) and the number was high enough int64 where it lost precision.

I mean, we can go at it all night long what pitfalls await in what language. Perhaps Rust is safest with its own pitfalls where you just can't do it safely (looking at you BST and use of Arc).

Programming is full of such traps and only inexperienced engineers in a language would make such a mistake. This includes engineers with 20+ years of 1 year experience.

In this case, a language that doesn't support automatic numeric type conversions. For instance, in Swift, 1 / FRAMERATE would give you integer division if FRAMERATE was an Int, or Double/Float division if FRAMERATE was a float, as the 1 literal would be inferred to a compatible type for / if one existed. You would never see Int / Float, or an implicit conversion between numeric types.
Never worked with Swift before, but this checks out: https://swiftfiddle.com/mt5uptmbynbt7bi4jot7m5laum

Careful though, if you don't have :Float on ts it still gives 0.

I don't understand the point of both replies... my point was that integer based division is preferred in all languages and the floating point must be somehow "forced" that one operand is float.

The post I was replying to mentions that this sort of problem does not exist in "better languages" but my point was that it does.

The problem here is that the code assigns an integer (result of division) to a float which is implicit "upgrade" in languages like C and C++ and required to be explicit in newer languages like Golang and Rust.

My point, which I seem to have failed to make, is that careless programmers would (and do!) make a silly thing like:

v := float64(operand1 / operand2)

just to satisfy the compiler error.

A common mistake for junior programmers but unforgivable (for some interpretations of unforgivable :) ) one for a senior.

> integer based division is preferred in all languages

What does this even mean?

First of all, several languages have distinct "integer divison" and "floating point division" operators, so there's no sense in which integer is "preferred" in those languages, they're unrelated operations.

Even allowing for your ignorance of such languages many modern languages do not have untyped constants, they're an attractive nuisance. If you don't have untyped constants then even if you're relying on implicit typing for constants (which I also don't like) you trip a mistake in the original expression anyway which is now unalike.

This mistake only occurs in a language with all of:

1. A single division operator despite two distinct operations

2. Untyped constants

3. "Promotion" so that type mismatches just do something unexpected and compile anyway.

Or just use 1.0, which can only be a Float literal.
Python (3) gets this right. Integer division is a separate operator: // vs /.
“only inexperienced engineers in a language would make such a mistake” — famous last words…
I'd say the real issue is reusing the same division operator for integers and floats. Integer division is an ugly operation (aside from powers of two) because it is extremely slow (for non-constant denominator) and has a uniquely high loss of information. It is a footgun even by itself.
At this point I've fixed enough bugs similar to this to do:

    float ts = 1.0f / (float)FRAMERATE;
I would still go further in modern times (C++23) and use the compiler to ensure fixed width types [1]:

    #define _float std::float32_t 
    _float ts = 1.0f / (_float)FRAMERATE;
Annoyingly though printing these numbers requires a cast which is not ideal.

[1] https://en.cppreference.com/w/cpp/types/floating-point

That code looks weird to me either way. You end up with a strange float. I represent framerates as rational numbers, e.g. 60/1. It works great for ffmpeg.
The Kalman filter code does need floats, though. So here we did indeed need a conversion.
Sure. I haven't looked at the code so I could easily be off base but, even with floats I want to delay that divide for as long as possible
Author here, totally agree with you. Changing the preprocessor directive was just the way I fixed it way back when.

Of course, the real fix is to use a proper constant.