inline methods and code readability in modern C++

Question

Alright, so sometimes my "coding brain" skips a gear; once in a while you can hear the gears grind. (For instance, every once in a while I write class Foo : Bar {} before reminding myself that's not proper anymore -- and hasn't been in a long time).

My current MO is to use inline methods as a way to improve code legibility and maintainability without sacrificing speed, but I ran across an issue recently that made me question this practice.

So, given (the admittedly contrived) code like:

double a;
double b = 0.0;
double c = 0.0;
...
// do some stuff here
...
// skip the sanity checks
// Magic Formula. This does what?
a = b + c - (b * c); 
...

I will write:

double a;
double b = 0.0;
double c = 0.0;
...
// do some stuff here
...
// skip the sanity checks
// Oh! It's probability!
a = ProbabilisticOr(b, c);
...
inline double ProbabilisticOr(double b, double c)
{
   // Skip the Sanity checks
   return b + c - (b * c);
}

The math I'm working on right now is fairly complex. If I want a generic CS/CE to be able to maintain it, it has to be written more like the second. The code is also pretty time sensitive.

I've recently run across an issue, as I said above. I made my mathematical constants static const double ... like a good little programmer; but when trying to access them inline the compiler bombs out for DLLs. The target OS is Linux, but I'm developing on Windows (Visual Studio 2013) and would like to keep it "cross-platform safe".

The solution to this little problem is to take them out-of-line; but, will that hurt my performance? Given the esoteric math involved, readability is a serious issue; but it still has to perform well.

Update:

To clarify, using more / different -- and much more contrived -- code:

#ifndef BUILD_DLL
#  define DLL_MODE __declspec(dllimport)
#else
#  define DLL_MODE __declspec(dllexport)
#endif


class DLL_MODE ContrivedProbabilityExample
{
public:
   inline ContrivedProbabilityExample(double value);
   inline ContrivedProbabilityExample& operator+=(double value);
private:
   inline void CheckValue(double value);
private:
   static const double ZERO_PROB = 0.0;
   static const double GUARANTEED_PROB= 1.0;
   double probability;

private:
   // Not implemented
   ContrivedProbabilityExample();
};

inline ContrivedProbabilityExample::ContrivedProbabilityExample(double value) : probability(value) 
{
   CheckValue(value);
}

inline ContrivedProbabilityExample& ContrivedProbabilityExample::operator+=(double value)
{
   CheckValue(value);
   probability = probability + value - (probability * value);
}

inline void ContrivedProbabilityExample::CheckValue(double value)
{
   if(value < ZERO_PROB || value > GUARANTEED_PROB) 
      throw std::range_error("Hey, whattaya think you're doing?");
}

This code will work fine in Static on both platforms; it will work as a Shared library on Linux. It will give an error under Windows when trying to use it as a DLL. The only solution is to move the CheckValue method out-of-line.

"old-school" inline had the CheckValue method's code being substituted "as-is" where it was called from; apparently "new-school" inline does ... nothing? (Since the compiler apparently does what it wants regardless.)

AFIK The only way to make this work under DLL is to move CheckValue out-of-line ... which could be a problem for time-sensitive code "old-school" (every call is/was guaranteed function overhead). Is this still a problem? Is there a "better" way to keep this readable; e.g. to not assume that every CS/CE working on my code will be proficient in Statistics?

Note: This is cross-platform, so "the compiler" may not be a meaningful phrase.

You define three variables in the same line without initializing any of them? I wouldn't recommend that. — Daniel Daranas, Jul 02 '14 at 15:43
Apart from that, I consider your question too broad. I think you should focus on explaining what is your exact problem. — Daniel Daranas, Jul 02 '14 at 15:45
keep in mind that `inline` is more like a suggestion for the compiler, the compiler can easily ignore that suggestion. Also I think that the default optimization on modern compilers are pretty good and sometimes you can even benefit from the absence of that keyword . — user2485710, Jul 02 '14 at 15:45
John - have you tried `constexpr`? It's intended to solve (or at least help with) the ODR issue for non-integral constants. — ecatmur, Jul 02 '14 at 15:47
@ecatmur ok, but I wasn't talking about ODR, I can't see how your comment relates to my comment. — user2485710, Jul 02 '14 at 15:51
Where are the `static const double ...` that are causing the problem? And where are you accessing them. (Making something `static` in a header can cause problems. But until we see exactly what you're doing, it's impossible to say.) — James Kanze, Jul 02 '14 at 16:12
@ecatmur anyhow, my point is that if you are writing code for performance you probably shouldn't rely on the `inline` keyword too much, reading your compiler documentation is a far better choice than using that keyword in my opinion. — user2485710, Jul 02 '14 at 16:19
I updated with an example to show the `static const double` issue I'm talking about. — John Price, Jul 02 '14 at 17:42
@DanielDaranas I didn't think it was relevant to the question, but I fixed that. — John Price, Jul 02 '14 at 17:44
@ecatmur Just looked it up. Apparently [constexpr and Visual C++ don't mix well](http://stackoverflow.com/questions/22546369/constexpr-wont-work-using-visual-c-compiler-nov-2013-ctp-ctp-nov2013) — John Price, Jul 02 '14 at 18:23
If performance is your concern, inlining may be solving a non-problem. You're not alone - I see it all the time on SO. If you've got general performance-related coding habits, that's fine, but the *effective* way to deal with performance is head-on. I use the [*random-pausing*](http://stackoverflow.com/a/378024/23771) method. — Mike Dunlavey, Jul 02 '14 at 18:41

score 1 · Answer 1 · answered Jul 02 '14 at 15:49

1

You want to use constexp, something like:

inline constexpr double ProbabilisticOr(double b, double c)
{
    // Skip the Sanity checks
    return b + c - (b * c);
}

then you're free to do things like:

static const double a = ProbabilisticOr(b, c);

answered Jul 02 '14 at 15:49

Paul Evans

26,111
3
30
50

Danvil · Answer 2 · 2014-07-02T17:58:17.933

Not sure what your question is, but what about this:

class DLL_MODE ContrivedProbabilityExample
{
public:
   ContrivedProbabilityExample(double value)
   { CheckValue(value); }

   ContrivedProbabilityExample& operator+=(double value)
   {
       CheckValue(value);
       probability = probability + value - (probability * value);
   }

private:
   void CheckValue(double value)
   {
       if(value < ZERO_PROB || value > GUARANTEED_PROB) 
           throw std::range_error("Hey, whattaya think you're doing?");
   }

private:
   constexpr double ZERO_PROB = 0.0;
   constexpr double GUARANTEED_PROB= 1.0;
   double probability;

private:
   // Not implemented
   ContrivedProbabilityExample();
};

score 0 · Answer 3 · answered Jul 02 '14 at 18:31

If your problem is ensuring performance, the best way to handle this is to insert an assert() to perform the sanity check.

It should be up to the caller to ensure that proper values are passed to your probability functions (and should be well documented), and an assert() will help you debugging if some caller doesn't. However, when you ship your code, you can simply deactivate all asserts, removing the performance penalty of the check.

There is nothing faster than not checking at all, that's why dereferencing pointers in C/C++ has never been safe and never will.

inline methods and code readability in modern C++

3 Answers3