-3

I am working on a mathematical vector.

I usually inline these types of class function members:

float getY() const { return m_y; }

But are these suitable to be inlined?

float getLength() const { return sqrt(m_x * m_x + m_y * m_y); }  
bool isUnitVector() const { return getLength() == 1.0f; }
Vector2D getZeroVector() const { return Vector2D(); } 
Bob
  • 327
  • 3
  • 12
  • 2
    What kind of answer do you expect? Apart it being quite opinion-based, I somehow doubt that either "No" or "Yes" would satisfy you. So in that sense it is unclear what you are asking (for). – Yunnosch Jun 19 '18 at 05:14
  • 1
    It would depend on the compiler being used and what optimizations are enabled when building the code as well as the context the code is used in. Also personal preference. So, "maybe". – Jesper Juhl Jun 19 '18 at 05:16
  • 3
    `... == 1.0f`. Even something that _is_ a unit vector empirically would probably be 1 to 4 ULPs away. This will often return `false` when you want it to return `true`. – Patrick Roberts Jun 19 '18 at 05:19
  • @Yunnosch he is talking about coding conventions/style – Ashvin Sharma Jun 19 '18 at 05:24
  • Adding to the comment by Patrick Roberts, have a look at this important problem discussion: https://stackoverflow.com/questions/588004/is-floating-point-math-broken – Yunnosch Jun 19 '18 at 05:26
  • @AshvinSharma How do you know? And how is it relevant for my question? I.e., what is your point? – Yunnosch Jun 19 '18 at 05:27
  • I know reading "But are these suitable to be inlined? ". It is relevant because I am telling you what kind of answer he expects. – Ashvin Sharma Jun 19 '18 at 07:13

3 Answers3

1

It's a matter of personal preference.

I like to do it this way but most workplaces don't like it.

The google coding standards suggest not to do them if they are not small but as I said some workplaces are absolutely pedantic about this.

https://google.github.io/styleguide/cppguide.html#Inline_Functions

solarflare
  • 423
  • 3
  • 14
1

There are several criterias you might use to decide if a method should be inlined or not:

  • Some recommend to inline getter functions only.
  • Another recommendation is to inline only short functions (e.g. up to 3 lines)
  • Performance: Compiler may be able to optimize better if a function is inlined.

On the other hand, there are some reasons to not inline too much:

  • More code in the header file leads to more changes of the header files, more dependencies and longer compilation times.
  • Inlined functions give a away the functionality of a module which you might want to hide (library internals).
  • Especially when you combine the declaration and definition of functions in the class declaration, it makes the class declaration longer and harder to read. (Better: Inline functions after the class declaration).

As others said, this is very opinion based. I recommend that you make up your mind about how You want to handle it, and then be consistent about it.

Rene
  • 2,466
  • 1
  • 12
  • 18
-1

That depends on how readable you want your code to be. For instance,

getLength()

is easier read and understood by another programmer -- and by yourself five years from now -- than

sqrt(m_x * m_x + m_y * m_y)

Of course one can make sense of it, but which one will take less time and make maintenance and development easier in the future?

What's more, with modern C/C++ compilers you shouldn't worry about inlining it, because they'll make the decision for you based on an analysis that's beyond my ability to explain. As you probably know, the inline keyword is, like the Pirates' Code, more of a guideline than a strict rule.

John Perry
  • 2,497
  • 2
  • 19
  • 28
  • Wut? The member is still there, and it's still being called. Whether or not the compiler inlines it, programmers will always see `getLength()`. The OP isn't asking whether or not they should ditch the member altogether. – StoryTeller - Unslander Monica Jun 19 '18 at 05:35
  • 1
    @StoryTeller It looks like JohnPerry is talking about writing `sqrt(m_x * m_x + m_y * m_y)` all over the place instead of `getLength()`. In other words, literally inlining the function definitions. – Justin Jun 19 '18 at 05:36
  • @Justin - I think we may have a breakdown in communication. AFAIK, to inline a function is to write it in a class definition (or with the `inline` keyword in a header, same thing). And that's what the OP seems to be talking about. Hence I'm curious at the first point presented here. – StoryTeller - Unslander Monica Jun 19 '18 at 05:38
  • @StoryTeller Yes. You are correct, of course. However, I'm merely restating / clarifying JohnPerry's point – Justin Jun 19 '18 at 05:41
  • @StoryTeller Justin is correct (thanks, Justin!); I was talking about manually inlining the functions, which is something people do sometimes, especially since the `inline` keyword doesn't exist in many languages and, as noted, is only a suggestion in C++, not a command. In fact, at first I started to write a reply based on the `inline` keyword, but the more I reread the OP's question, the more I thought he meant manual inlining, because the `inline` keyword doesn't appear in his question. – John Perry Jun 19 '18 at 07:41
  • @StoryTeller To wit, I have sometimes manually inlined statements that gcc or clang refuse to inline even when the `inline` keyword is present, observing a corresponding improvement in code. – John Perry Jun 19 '18 at 07:43
  • I see. So It was a breakdown in communication. `inline` may be like the Pirates' Code (great reference BTW) when it comes to actual inlining, but it does have linkage implications. So it's not entirely pointless to consider. As for manually inlining, I haven't personally had the need, so I can't comment. – StoryTeller - Unslander Monica Jun 19 '18 at 07:45
  • @StoryTeller Yes, optimizing is sometimes strange. I once had something like `a.b[i] = ...` in a computationally intensive loop, where the struct `a` and the array `b` were never changed, and neither gcc nor clang noticed that they didn't have to constantly offset from `a`'s base address. I managed a 30-40% speedup in the code by simply assigning `b = a.b` outside the loop, then assigning to the `b[i]` instead of to `a.b[i]`. Meanwhile the compilers do all sorts of complicated things that don't seem to help at all. Sheesh. – John Perry Jun 19 '18 at 07:50
  • Seems like a missed optimization opportunity. I know both Clang and GCC's bug trackers have a heap of those being reported. But I guess no matter how good they get, there is no replacement for profiling an examining the outputted assembly. – StoryTeller - Unslander Monica Jun 19 '18 at 07:52
  • (You may notice some edits: that's because I looked up an email where I described it to someone. Sorry for any confusion.) – John Perry Jun 19 '18 at 07:53
  • NP. I'm guilty of ninja-editing myself more often than not. – StoryTeller - Unslander Monica Jun 19 '18 at 07:54
  • If the compiler doesn't inline a function that you want it to inline, don't remove the function and manually inline all over (and don't use a macro to automate this). Prefer decorating the function with `__attribute__((always_inline))` or in C++11 `[[gnu::always_inline]]` (MSVC is `__force_inline` or smth like that. Sadly, they don't have a `[[msvc::force_inline]]`). Alternatively, if you can afford it, profile guided optimization can often pick up stuff like this. – Justin Jun 19 '18 at 17:33
  • @Justin Thanks, but does that work in clang, which I use about as often as g++? I try to write pretty standard and portable code, having dealt with too many C++ programs that were written 1-2 years before (or more) which don't compile on the latest compiler because they used some trick or other to speed things up. I do profile; that's how I stumbled on the issue with `a.b[i]`. In any case, that's beside the original point, which is whether my answer was relevant to the OP's question. – John Perry Jun 19 '18 at 20:18
  • 1
    @JohnPerry Yes, clang supports `[[gnu::always_inline]]` (at least IIRC). At any rate, clang does support `__attribute__((always_inline))`. The benefit of the C++11 attribute is that it's portable; if a compiler doesn't recognize `[[gnu::always_inline]]`, it has no effect. What you *can* do is use macros and `#if` to `#define MY_LIB_ALWAYS_INLINE` to an appropriate attribute for the particular compiler – Justin Jun 19 '18 at 20:34