isocpp / CppCoreGuidelines

The C++ Core Guidelines are a set of tried-and-true guidelines, rules, and best practices about coding in C++
http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines
Other
42.39k stars 5.42k forks source link

I.4 : Math meets real world #1221

Open imre-palik opened 6 years ago

imre-palik commented 6 years ago

This recommendation tends to break down when math meets real world. Let's say, you have a mathematics library (types: float, double, long double), but your code deals with objects of distance, time, speed, etc. The convenience & speed of the mathematics library tends to make strongly typed interfaces impossible.

xtofl commented 6 years ago

We wrote code against e.g. thrust that proves this wrong. It isn't all that hard to write a class that is aware of what physical property it represents, and offers a convenient interface that adapts to such libraries.

I think indeed, though, this guideline would benefit from such an example.

imre-palik commented 6 years ago

Could you provide an example? I wonder if it would be applicable to the issues I tend to encounter.

xtofl commented 6 years ago

In our case, we had an API defining functions accepting a matrix of floats. Each function

All matrix dimensions were provided as ints, and the row/column majority was managed 'by hand'.

We found ourselves making too many errors, so reworked the interface to work on types

This refactoring revealed more than three bugs that would long have gone unnoticed. The bugs exposed themselves as compilation errors - code that would not have been written.

When I have time, I'll write up a decent example.

hsutter commented 6 years ago

Editors call: We don't understand how the recommendation breaks down. For example, a wrapped integer is as efficient on real world compilers as a plain integer.

@xtofl: Yes, please do write up an example that we can add. If it is short and easy to understand we could add it to the item, otherwise at least it will be captured in this comment thread where it can be referred to.

imre-palik commented 6 years ago

The issues are the following: Common mathematical libraries often have C interfaces, i.e. float* stuff.

Wrapping the whole library is often not an option, as both management, and the domain experts who actually write most of the code that uses it tend to frown on it.

Then unwrapped types spread out in the codebase.

scraimer commented 6 years ago

The point of I.4 is to encourage use of the powerful static type system of C++, which allows discovering preventable bugs at compile-time. While wrapping a library into that type system comes with a development cost, it is usually worth it.

Wrapping the whole library is often not an option, as both management, and the domain experts who actually write most of the code that uses it tend to frown on it.

It sounds like it was an option, just one that you weighed and dismissed. Presumably because the cost of training and development outweighed the potential cost of bugs that would be prevented by wrapping the library.

That's fine. You (or management) made a choice. The guidelines aren't meant to substitute for your opinions. You clearly know what's best for you. But if you want this to be encoded in the guidelines, you have to demonstrate that what's best for you is best for everyone. I'm not convinced yet. For example:

Then unwrapped types spread out in the codebase

This sounds to me like a great place where you could have wrapped the types in more C++-style types - before they spread out into the codebase. You mentioned float *, that sounds like a prime candidate for RAII for releasing resources upon destruction