overloading the + operator

[parallx]

Hi

I need to overload a + operator to assign memory in a 1D array of pointers.

operator+: This operator adds a team of gladiators by receiving an array of integers.
The integer array will contain values 1 to 4, representing Trax, Murmillo, Retiariusand Dimachaerus gladiators respectively. You must insert the type of gladiator specied at the array index into the fightingTeam variable. Remember that the memory will have to be allocated freshly. You can assume the passed in team will match the size of the teams already in use.

Here is what i've got so far but my lack of experience is making this tricky:

team& team::operator+(int *add) {
    const int trax_number = 1;
    const int murmillo_number = 2;
    const int retiarius_number = 3;
    const int dimachaerus_nmuber = 4;

    team **arrayTeam = new team*[teamSize];
    for (int i = 0; i < teamSize ; ++i) {
        arrayTeam[i] = *add;
    }
}

Can anyone help ?

[Repeater]

You say

assign memory in a 1D array of pointers

but then

an array of integers

. Which is it? Are you work with an array of pointers, or an array of int?

[parallx]

Repeater the spec given by the lecturer is trash but I got a working solution.

Thanks

[keskiverto]

Lets comment on the code nevertheless:

team& // should return a reference, but function has no return statements
team::operator+(int *add) {
// operator is a member.  Would be used:
// team foo;
// int* bar;
// x = foo + bar;

  // these constant are not used in this function
    const int trax_number = 1;
    const int murmillo_number = 2;
    const int retiarius_number = 3;
    const int dimachaerus_nmuber = 4;

  // teamSize is member of this team
    team **arrayTeam = new team*[teamSize];
  // address of allocated block won't be known elsewhere => leak

  // same value is copied to every element of an array
    for (int i = 0; i < teamSize ; ++i) {
        arrayTeam[i] = *add; // int is not a team*
    }
}

Unnecessary code, missing code, type mismatches, memory leak, ...

That, and your lack of experience, makes one wonder whether the "working solution" is correct and not merely something that appears to work with limited testing.


A standalone function could be simple:

T operator+ ( T lhs, const U & rhs ) {
  lhs += rhs;
  return lhs;
}

And have a "symmetry pair":

T operator+ ( const U & lhs, T rhs ) {
  rhs += lhs;
  return rhs;
}

But then you need a different member:
T::operator+= ( const U & )

[parallx]

My original option was full of errors indeed but after consulting multiple sources I created the following which was marked correct by the auto marker I had to upload to:

team& team::operator+(int *add) {//allocates memory and creates a new derived object class.
    fightingTeam = new gladiator*[teamSize];
    for (int i = 0; i < teamSize ; ++i) {
        switch (add[i]){
            case trax_number:
                fightingTeam[i] = new trax();
                break;
            case murmillo_number:
                fightingTeam[i] = new murmillo();
                break;
            case dimachaerus_number:
                fightingTeam[i] = new dimachaerus();
                break;
            case retiarius_number:
                fightingTeam[i] = new retiarius();
                break;
            default:
            {
                cout<<"Invalid gladiator type"<<endl;
            }
        }
    }
    return *this;
}

Once I actually understood exactly what had to be done, I had a better idea of what to do but as usual I was overcomplicating the matter.

[Ganado]

Still looks like quite a few memory leaks if you call that twice in a row.

[keskiverto]

Indeed.

team::foo()
{
  // what if the fightingTeam already points to an array here?
  // the assignment below forgets (leaks) that array
  // and all the gladiators pointed to by that array
  fightingTeam = new gladiator*[teamSize];
  // more code
}

Furthermore, I do suspect that at least the copy constructor, copy assignment, and destructor of class team might leak.
https://en.cppreference.com/w/cpp/language/rule_of_three


Edit:

team& team::operator+ ( int *add )
{
    for ( int i = 0; i < teamSize ; ++i) {
        switch ( add[i] ) {

That loop assumes that the add points to an array that has at least teamSize elements. Nothing checks or ensures that. This function has blind faith to the caller.

    fightingTeam = new gladiator*[teamSize];
    for (int i = 0; i < teamSize ; ++i) {
        switch (add[i]){
            default:
            {
                cout<<"Invalid gladiator type"<<endl;
            }
        }

The elements of the array are uninitialized. If any add[k] falls to the default, then the corresponding fightingTeam[k] will remain uninitialized.

I bet that the other team functions assume each fightingTeam to have a valid pointer.

marked correct by the auto marker

Well, at least we now know not to trust it. At all.

[parallx]

The spec doesn't ask for any checks. It only asked for checks in the deallocating function. I get what you are all saying but the spec is 13 pages and I don't think anyone is keen to read that. But it had a whole bunch of things you can ignore which is dumb but there's not much I can do.