5
\$\begingroup\$

I'm implementing a basic array data structure with basic functionalities.

#include <iostream>
#include "barray.h"

BArray::BArray(int init_size)
{
    b_array = new int[init_size]();
    array_size = init_size;
}

BArray::BArray(int init_size, int init_val)
{
    b_array = new int[init_size];
    array_size = init_size;

    for(int i = 0; i < init_size; ++i)
        b_array[i] = init_val;
}

BArray::BArray(const BArray & rhs)
{
    array_size = rhs.array_size;
    b_array = new int[array_size];

    for(int i = 0; i < array_size; ++i)
        b_array[i] = rhs[i];
}

BArray::~BArray()
{
    delete [] b_array;
}

int BArray::getSize() const
{
    return array_size;
}

int BArray::operator[](int index) const
{
    return *(b_array + index);
}

int& BArray::operator[](int index)
{
    return *(b_array + index);
}

BArray& BArray::operator=(const BArray& rhs)
{
    if(this == &rhs)
        return *this;

    array_size = rhs.array_size;

    delete [] b_array;

    b_array = new int[array_size];

    for(int i = 0; i < array_size; ++i)
        b_array[i] = rhs[i];

    return *this;
}

std::ostream& operator<< (std::ostream& out, const BArray& arr)
{
    for(int i = 0; i < arr.array_size; ++i)
        out << arr[i] << " ";

    return out;
}

And the header file

#ifndef BARRAY_H
#define BARRAY_H

class BArray
{
public:
    BArray() = delete;                    //Declare the default constructor as deleted to avoid
                                          //declaring an array without specifying its size.
    BArray(const int init_size);
    BArray(int init_size, int init_val);  //Constructor that initializes the array with init_val.
    BArray(const BArray & rhs);           //Copy constructor.
    ~BArray();                            //Destructor.


    int operator[](int index) const;      //[] operator overloading for "reading" index value.
    int& operator[](int index);           //[] operator overloading for "setting" index value.
    BArray& operator=(const BArray& rhs); //Copy assignment operator.

    //Utility functions.
    int getSize() const;

    //Friend functions.
    friend std::ostream& operator<< (std::ostream& out, const BArray& arr);

private:
    int* b_array;
    int array_size;
};



#endif // BARRAY_H

Can you please give me a feedback on what is missing, what is wrong and what is good? I mean in terms of memory allocation, operators overloading, etc...

Is this is the best way this class can be implemented?

Edit: This is how I tested the code

#include <iostream>

#include "barray.h"

int main()
{

    //BArray invalid_instance; //Default constructor is deleted.

    BArray barr(5); //Declaring an array of size 5, this initializes all values to zeros.

    std::cout << barr[2] << std::endl;

    barr[2] = 15;   //Setting index 2 to 15.

    std::cout << barr[2] << std::endl; //Reading out value of index 2.

    BArray anotherArray(barr); //Copy constructor.

    std::cout << anotherArray[2] << std::endl;

    anotherArray[3] = 8;

    BArray assignArray = anotherArray; //Copy assignment operator.

    std::cout << assignArray[2] << std::endl;

    std::cout << assignArray; //Printing out array values.

    return 0;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Do you have some test code that you add to the question to show us how the class is used? \$\endgroup\$ – pacmaninbw May 14 at 23:00
  • 1
    \$\begingroup\$ Yes sure. I will add it now. \$\endgroup\$ – Bilal Ahmed May 14 at 23:02
  • \$\begingroup\$ How's that a "static" array? \$\endgroup\$ – L. F. May 15 at 9:46
  • \$\begingroup\$ @L.F. By static I mean not dynamic/resizeable. \$\endgroup\$ – Bilal Ahmed May 15 at 20:34
4
\$\begingroup\$
  1. The default-ctor won't be implicitly declared as there are user-declared ctors.

  2. When you can define a default-ctor with reasonable behavior, consider doing so.
    If you use in-class initializers to 0 resp. nullptr for the members, it can even be explicitly defaulted, making the class trivially default-constructible.

  3. Top-level const on arguments in a function declaration is just useless clutter.

  4. Consider investing in move-semantics to avoid costly copies.

  5. If the allocation in one of the ctors throws, your dtor will be called on indeterminate members, which is undefined behavior.
    Use mem-initialisers or pre-init b_array to nullptr to fix it.

  6. Your copy-assignment also has pathological behavior in the face of exceptions. That aside, it pessimises the common case in favor of self-assignment. Read up on the copy-and-swap idiom. As a bonus, you get an efficient swap() out of it.

  7. Using a std::unique_ptr for the member would allow you to significantly simplify the memory-handling.

  8. Keep to the common interface-conventions. Failure to follow it makes generic code nigh impossible. Specifically, .getSize() should be .size().

  9. You are missing most of the expected interface, specifically iterators (normal, constant, reverse, related typedefs, .data()).

  10. Better names for the private members would be array and count.

\$\endgroup\$
4
\$\begingroup\$

I would allow the default constructor and make it default to a 0 size array. with the optimization that b_array is a nullptr.

Then you can also add move constructors/assignment. Where the moved-from object gets a b_array = nullptr;

On of the basic functionalities of a container is iteration. So you should add a begin() and end() that return the b_array and b_array+array_size resp. Don't forget the const versions.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.