Encapsulation: public member vs public method

Question

Suppose I have a class called PointerSet<T>. It is essentially a vector that acts like a set through use of std::lower_bound (but how it works isn't important so I won't go into further detail).

template<typename T>
class PointerSet
{
    ...

public:
    void insert(T&);
    void erase(T&);
    void erase(const std::vector<T>&);
};

Will having a "function tunnel" take a hit to performance? (I don't know the technical term, but it is what I call it in my mind. It is a function calling a function with the exact same purpose in mind.) Please see Version 1.

Version 1: Create a member method

...

template<typename T>
class Node
{
    PointerSet<T> Links;
public:
    void insertLink(T& p){ Links.insert(p); }
    void eraseLink(T& p){ Links.erase(p); }
    void eraseLink(const std::vector<T>& p){ Links.erase(p); }
};

class bar;
class foo : public Node<bar> { };    // now foo can insert links to bars.

...

Version 2: Just use a public member

...

class bar;
struct foo
{
    PointerSet<bar> Links;    // Use Links directly
};

Which of these is the best approach? I am thinking about performance and ease of debugging.

Well, if the implementation is a `vector` that act like a `set` then your public interface would make it easy to *incorrectly* use your `PointerSet` class. Assing a lot of items in a vector and required them to be sorted would be very inefficient for large data as you would always copy a lot of data when inserting or removing an item. One possibility would be to implement an heap. Except if you have a large number of small items and do either an heap or ensure the data is sorted only once the data is loaded (and items are not removed) then it might make sense if you have a performance problem. — Phil1970, Jun 21 '17 at 01:18
@Phil1970 I thought so as well, but I have noticed in actual performance tests that even on some very large data, the `vector` still outperforms the `set`. Until the data gets astronomically large, anyway. I guess move semantics have helped vectors be very fast at shifting elements. Perhaps you or someone could clarify this? I suppose that is a different question, though. — Dustin Nieffenegger, Jun 21 '17 at 01:34
Well, if performance really matters, then you should measure it anyway so the question would be pointless as you would know the real impact with your actual data and usage... and otherwise, you could assume that `std::set` is adequate and not reinvent the wheel. Also you have to be sure that you make the test with worst scenario. A vector would be faster up to a given size (that depend on item size and their copy/move implementation) and also actual hardware (cache size...) and mainly the usage pattern (ratio of insert/delete compared to number of searches). — Phil1970, Jun 21 '17 at 01:50
Possible duplicate of [Public Data members vs Getters, Setters](https://stackoverflow.com/questions/2977007/public-data-members-vs-getters-setters) — Piotr Siupa, Jun 21 '17 at 02:52

score 3 · Accepted Answer · answered Jun 21 '17 at 01:38

Will having a "function tunnel" take a hit to performance?

Most likely not. Especially since you are dealing with templates here, and so the definitions will all be visible and easy for the compiler to inline. Take a look at this link: https://godbolt.org/g/cWR7N3

What I've done is compiled these two code snippets. First, calling the functions from the Node class.

#include <vector>

// these functions are not defined, so that the compiler
// cannot inline them or optimize them out
void insert_impl(void const*);
void erase_impl(void const*);
void erase_impl_vec(void const*);

template<typename T>
class PointerSet
{
public:
    void insert(T& v) { insert_impl(&v); }
    void erase(T& v) { erase_impl(&v); }
    void erase(const std::vector<T>& v) {
        erase_impl_vec(&v);
    }
};

template<typename T>
class Node
{
    PointerSet<T> Links;
public:
    void insertLink(T& p){ Links.insert(p); }
    void eraseLink(T& p){ Links.erase(p); }
    void eraseLink(const std::vector<T>& p){ Links.erase(p); }
};

int main()
{
    Node<int> n;

    int x;
    n.insertLink(x);
    n.eraseLink(x);

    std::vector<int> v;
    n.eraseLink(v);
}

And then calling them directly from the PointerSet class.

#include <vector>

// these functions are not defined, so that the compiler
// cannot inline them or optimize them out
void insert_impl(void const*);
void erase_impl(void const*);
void erase_impl_vec(void const*);

template<typename T>
class PointerSet
{
public:
    void insert(T& v) { insert_impl(&v); }
    void erase(T& v) { erase_impl(&v); }
    void erase(const std::vector<T>& v) {
        erase_impl_vec(&v);
    }
};

int main()
{
    PointerSet<int> n;

    int x;
    n.insert(x);
    n.erase(x);

    std::vector<int> v;
    n.erase(v);
}

As you can see in the link (https://godbolt.org/g/cWR7N3), the compiler output identical assembly for each.

main:                                   # @main
        push    rbx
        sub     rsp, 48
        lea     rbx, [rsp + 12]
        mov     rdi, rbx
        call    insert_impl(void const*)
        mov     rdi, rbx
        call    erase_impl(void const*)
        xorps   xmm0, xmm0
        movaps  xmmword ptr [rsp + 16], xmm0
        mov     qword ptr [rsp + 32], 0
        lea     rdi, [rsp + 16]
        call    erase_impl_vec(void const*)
        mov     rdi, qword ptr [rsp + 16]
        test    rdi, rdi
        je      .LBB0_3
        call    operator delete(void*)
.LBB0_3:
        xor     eax, eax
        add     rsp, 48
        pop     rbx
        ret
        mov     rbx, rax
        mov     rdi, qword ptr [rsp + 16]
        test    rdi, rdi
        je      .LBB0_6
        call    operator delete(void*)
.LBB0_6:
        mov     rdi, rbx
        call    _Unwind_Resume
GCC_except_table0:
        .byte   255                     # @LPStart Encoding = omit
        .byte   3                       # @TType Encoding = udata4
        .byte   41                      # @TType base offset
        .byte   3                       # Call site Encoding = udata4
        .byte   39                      # Call site table length
        .long   .Lfunc_begin0-.Lfunc_begin0 # >> Call Site 1 <<
        .long   .Ltmp0-.Lfunc_begin0    #   Call between .Lfunc_begin0 and .Ltmp0
        .long   0                       #     has no landing pad
        .byte   0                       #   On action: cleanup
        .long   .Ltmp0-.Lfunc_begin0    # >> Call Site 2 <<
        .long   .Ltmp1-.Ltmp0           #   Call between .Ltmp0 and .Ltmp1
        .long   .Ltmp2-.Lfunc_begin0    #     jumps to .Ltmp2
        .byte   0                       #   On action: cleanup
        .long   .Ltmp1-.Lfunc_begin0    # >> Call Site 3 <<
        .long   .Lfunc_end0-.Ltmp1      #   Call between .Ltmp1 and .Lfunc_end0
        .long   0                       #     has no landing pad
        .byte   0                       #   On action: cleanup

score 1 · Answer 2 · answered Jun 21 '17 at 00:52

1

Unless you would like to limit the operations that can be preformed on that data member, I would recommend using the public member. If all you are doing is wrapping the methods with your own method names, it doesn't make sense to have them there.

answered Jun 21 '17 at 00:52

LucasN

243
1
2
14

Well it really depend on the purpose of `foo` class. If it hold only a few variables, then a public member might be adequate. Otherwise better to write the few extra inline functions as it could made the maintenance easier if the implementation changes. – Phil1970 Jun 21 '17 at 01:25

score 0 · Answer 3 · answered Jun 21 '17 at 01:36

Generally, you should avoid public derivation if IS-A relationship is not respected as it cause an higher coupling that composition.
You should also avoid public member as it can make the application harder to maintain in the future.
If you could use PointerSet directly in your second sample, then in version 1, you could have derived directly from PointerSet too. Thus the comparison is unfair.

In practice, acceptable compromises depend a lot on the actual purpose of each class. Extra indirections are useless if they don't give any benefits (reduced compilation dependencies, access/visibility control, extra functionalities, code reuse opportunities...)

Encapsulation: public member vs public method

3 Answers3