質問

Imagine the following scenario:

class A 
{
    int a[50];
    int* GetAPtr() { return a; };
};
...
A b;
if(b.GetAPtr()[22] == SOME_RANDOM_DEFINE) do_this_and_that();

Is this kind of access considered bad practice? b.GetAPtr()[22]

To clarify my situation:
1. I cannot use new/malloc in this case, the array muste be static
2. This is meant to encapsulate older C code that uses multiple arrays where this comes extremly handy
3. I know that returning a pointer can possibly return a NULL pointer, we do not talk about that issue here

役に立ちましたか?

解決

If you really need such const expression you could make it into a function:

class A 
{
    int a[50];
    bool check_this_and_that() { return a[22] == SOME_RANDOM_DEFINE; };
};
...
A b;
if(b.check_this_and_that()) do_this_and_that();

magic numbers are bad in general but inside a class logic it's more forgiveable and outsiders don't have to see this.

他のヒント

Yes, it is bad practice, because you have no way of knowing how long the array is. You could follow the idiomatic standard library approach and return begin and end pointers, pointing to the first and one-past-last elements.

class A 
{
    int a[50];
    int* begin() { return &a[0]; };
    int* end() { return &a[50]; };
    const int* begin() const { return &a[0]; };
    const int* end() const { return &a[50]; };
    size_t size() const { return 50; } // this could be handy too
};

As well as giving you the tools to iterate over the elements like you would over a standard library container, this allows you to check whether any pointer to an element of the array is < v.end(). For example

it* it = b.begin() + 22;
if(it < b.end() && *it == SOME_RANDOM_DEFINE) do_this_and_that();

This makes it trivial to use standard library algorithms:

A b;
// fill with increasing numbers
std::iota(b.begin(), b.end());
// sort in descending order
std::sort(s.begin(), s.end(), std::greater<int>());

// C++11 range based for loop
for (auto i : b) 
  std::cout << i << " ";
std::endl;

GetAPtr is a method for accessing a private data member. Now ask yourself what are the advantages of b.GetAPtr()[22] over b.a[22]?

Encapsulating data is a good way to maintain constraints on and between data members. In your case there is at least a correlation between the a array and its length 50.

Depending on the use of A you could build a interface providing different access patterns:

class A {
   int a[50];
public:
   // low level
   int atA(unsigned i) const { return a[i]; }
   // or "mid" level
   int getA(unsigned i) const { if(i >= 50) throw OutOfRange(); return a[i]; };
   // or high level
   bool checkSomething() const { return a[22] == SOME_RANDOM_DEFINE; }
};
ライセンス: CC-BY-SA帰属
所属していません StackOverflow
scroll top