0

Example class I'm using:

class Vector
{
double val[3];
public:
double & operator [] (const unsigned int & index) {return this->val[index];};
}

Then I call it like:

Vector Example;
Example[0]=5;

Is using operator overloading like this correct or it is against encapsulation and I should use something different? I'm using reference to private value here and I'm not sure about this implementation.

Ava
  • 818
  • 10
  • 18
  • 2
    This is basically how the indexing of `std::vector` works. – Some programmer dude Mar 19 '18 at 12:43
  • 5
    Setter and getters are evil IMHO. This looks fine and has expected behavior. My only suggestion would be to write it as `double& operator[](std::size_t index) {return val[index];}`. Also note you don't need the `;` at the end of a inline function. – NathanOliver Mar 19 '18 at 12:46
  • once you return a non-const reference you can make the member public as well, but it depends on what you actually want to achieve – 463035818_is_not_an_ai Mar 19 '18 at 12:47
  • Is there an error with your current code? – Jake Freeman Mar 19 '18 at 12:54
  • Thank you for answers. There is no error, I just want to have good programming habits, that's why I asked. – Ava Mar 19 '18 at 13:01
  • Your operator[] is 100% appropriate for a vector object. But wait... there's more. See the answer I posted. – Jive Dadson Mar 19 '18 at 13:53
  • Better return a `const` reference, if you are going to return a reference to some private field. If you don't you run the risk of allowing other software to modify your instance internals and you probably break the software contract of your class. – Luis Colorado Mar 20 '18 at 09:13

2 Answers2

1

Good so far... You also need one that can read from const objects. Also, there's no reason to pass an array index by const&. Also also, this-> is implicit. Look at the member function signatures for std::vector<>. In particular operator[]. Push request...

class Vector
{
    double val[3];
  public:
    double& operator [] (size_t index) {return val[index];};
    const double& operator [] (size_t index) const {return val[index];};
};
Jive Dadson
  • 16,680
  • 9
  • 52
  • 65
0

This is a leak in your abstraction. It exposes the fact you have actual doubles that can be read from or written to.

If you later wanted to change those doubles into a remote immediate network connection data or stored in a database, you would be forced to add breaking changes to your interface.

That being said:

It is worth it. You probably will never modify this type to do something that insane, and there are significant compile, design and runtime overheads to unlimited abstraction.

Abstraction and encapsulation serve a purpose and have a cost.

std::vector<bool>'s operator[] is an example of what you can do when reference return types are not ok. There it is used to return sub-byte elements. Note that it is widely considered a design error.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Same leak in the std::vector abstraction. It allows statements like `vec[3] = 42;` – Jive Dadson Mar 19 '18 at 13:33
  • @JiveDadson vector leaks a lot more than that; it guarantees a contiguous buffer. – Yakk - Adam Nevraumont Mar 19 '18 at 13:46
  • 1
    So does the OP's. Those are Good Things, imo. – Jive Dadson Mar 19 '18 at 13:47
  • @JiveDadson No, the OP's API doesn't guarantee a continguous buffer. `class Foo { double v1, v2; int x; double v3; public: double& operator[](unsigned x) const { if (x==0) return v1; if (x==1) return v2; if (x==2) return v3; } };` -- same API, not a contiguous buffer. You cannot, however, provide that API without actually storing doubles. `vector` on the other hand has `T* data()` (plus other guarantees). Please note, I am not judging, I am describing. – Yakk - Adam Nevraumont Mar 19 '18 at 13:58
  • `double val[3]` is contiguous. That's all I meant. The sad truth is, there is no standard way to determine if a container uses contiguous memory. The committee is strugling with it. The design error in that case is that the iterator categories should not exist. Iterator traits (including is_contiguous_v, can_index_forward_v, etc.) would have been far preferable. – Jive Dadson Mar 19 '18 at 14:01