0

The Problem: When I attempt to assign an IntArray object by index I get the following error:

"Expression is not assignable."

The error is produced by the following line of code in iadrv.cpp:

IntArray a(10);
for(int i = a.low(); i <= a.high(); i++)
    a[i] = i * 10;

I am able to assign an entire IntArray object to another like so, a = b;, however when a specific index is referred to the "expression is not assignable" error occurs.

EDIT: I removed the const declaration from most of the functions and I am not getting the "Expression is not assignable" error anymore. However, the setName now gives the error:

"ISO C++ 11 does not allow conversion from string literal to 'char *'"

This error is cause by the following code in iadrv.cpp:

a.setName("a");

Program Explanation:

I have written a class IntArray (in C++) in which the following operators are overloaded:

  • "[ ]" : allows index range checking
  • "=" : allows array assignment
  • "+" : allows the sum of two arrays to be assigned to a third array
  • "+=" : allows the sum of two arrays to be assigned to the first array
  • "<<" : allows the contents of an array to be output

The program also includes functions:

  • setName : sets the name of the IntArray object
  • getName : returns the name of the IntArray object
  • low : returns the smallest legal index
  • high : returns the largest legal index
  • length : returns the number of elements

A driver program (iadrv.cpp, iadrv.h) will run tests on the IntArray class (IntArray.cpp, IntArray.h) to determine if all operators were properly overloaded.

Note: that for each array test data, the driver will simply multiply the array index by 10 immediately after each array is initialized or modified and output its contents. When the program encounters a run-time error, it should "simulate"a halt with appropriate diagnostics rather than actually halting the program.

The Code:

IntArray.h

//  IntArray.h

#ifndef __IntArray__IntArray__
#define __IntArray__IntArray__

#include <iostream>
#include <fstream>
#include <iomanip>

using namespace std;

class IntArray {
private:
    int a, b;
    int size;
    int * num;
    char * name;
public:
    IntArray(int start, int finish);
    IntArray(int finish = 10);
    IntArray(const IntArray &); //constructor copy
    ~IntArray();
    int low() const;
    int high() const;
    char * getName() const;
    //removed the const declaration from functions below
    int & operator [] (int);     //made to return int&
    friend ostream & operator << (ostream &, IntArray &);
    void setName(char *);
    int length() const;
    const IntArray & operator = (IntArray &);
    const IntArray & operator + (IntArray &);
    bool operator += (IntArray &);

};

#endif /* defined(__IntArray__IntArray__) */

IntArray.cpp

//  IntArray.cpp

#include "IntArray.h"

#include <iostream>
#include <fstream>

using namespace std;

extern ofstream csis;

IntArray::IntArray(int start, int finish) {
    if (start > finish) {
        cout << "Simulating a halt.";
        a = finish;
        b = start;
    }
    else {
        a = start;
        b = finish;
    }
    size = b-a;
    num = new int[size];
    name = new char[1];
    for (int i = 0; i < size; i++) {
        num[i] = 0;
    }
}
IntArray::IntArray(int finish) {
    size = finish;
    a = 0;
    b = finish - 1;
    num = new int[size];
    name = new char[1];
    for (int i = 0; i < size; i++) {
        num[i] = 0;
    }
}
IntArray::IntArray (const IntArray & right): size(right.size) {
    num = new int[size];
    name = new char[1];
    for (int i = 0; i < size; i++) {
        num[i] = right.num[i];
    }
}
IntArray::~IntArray() {
    delete[] num;
    delete [] name;
}
int IntArray::low() const{
    return a;
}
int IntArray::high() const{
    return b;
}
char * IntArray::getName() const{
    return name;
}
void IntArray::setName(char * n) {
    name = n;
}
//removed const declarations
//made to return int&
int & IntArray::operator [] (int subscript) const{
    if (subscript < a || subscript > b) {
        cout << "subscript: " << subscript << endl;
        cout << "Out of bound error. Simulating a halt." << endl;
        return num[a];
    }
    return num[subscript-a];
}
int IntArray::length() const{
    //b-a = size
    return (b-a);
}
//removed const declarations
ostream & operator << (ostream & output, IntArray & array) {
    for (int i = array.low(); i <= array.high(); i++) {
        output << array.name << "[" << i << "] = " << array[i] << endl;
    }
    return output;
}
//removed const declarations
IntArray & IntArray::operator = (IntArray & right) {
    if (length() == right.length()) {
        for (int i = 0; i <= length(); i++) {
            num[i] = right[right.low()+i];
        }
    return * this;
    }
    else {
        delete [] num;  //reclaim space
        delete [] name;
        size = right.length();
        num = new int [size]; //space created
        cout << "Different sized arrays. Simulating a hault" << endl;
    }
    return * this;
}
//removed const declarations
IntArray & IntArray::operator + (IntArray & right) {
    int * ptr;
    ptr = new int [right.length()];
    if (length() == right.length()) {
        for (int i = 0; i < length(); i++) {
            ptr[i] = num[i] + right[right.low()+i];
        }
    }
    return * this;
}
//removed const declarations
bool IntArray::operator += (IntArray & right) {
    if (length() == right.length()) {
        for (int i = 0; i <= right.length(); i++) {
            num[i] += right[right.low()+i];
        }
        return true;
    }
    cout << "Could not add the sum of the arrays into first array. Simulating a halt." << endl;
    return false;
}

iadrv.h

//  iadrv.h

#ifndef p6_iadrv_h
#define p6_iadrv_h

#include "intarray.h"

int main();
void test1();
void wait();

#endif

iadrv.cpp

//  iadrv.cpp

#include <iostream>
#include <iomanip>
#include <fstream>
#include <stdlib.h>
#include "iadrv.h"

using namespace std;

ofstream csis;

int main() {
    csis.open("csis.dat");
    test1();
    csis.close();
}

void test1() {
    system("clear");
    cout << "1. Array declared with single integer: IntArray a(10);" << endl << endl;
    csis << "1. Array declared with single integer: IntArray a(10);" << endl << endl;
    IntArray a(10);
    for(int i = a.low(); i <= a.high(); i++)
        a[i] = i * 10;
    a.setName("a");
    cout << a << endl;
    csis << a << endl;
    wait();
}

DISCLAIMER: This program was written as a school assignment and has already been turned in to be graded. This was my first c++ program so I would like to understand my mistakes. Your help is sincerely appreciated.

Improve this question
1
  • Your operator= is too complex. It could be written much more simpler than what you attempted. In addition, your operator+ could be written to just call operator +=. Commented Nov 21, 2014 at 4:27

3 Answers 3

Reset to default
1

You have defined your operator[] like this:

const int operator [] (int) const;

that second "const" means that inside that method you cannot modify your object.

So it will only work for getting values, but not for setting them.

Try to remove it and it should work.

EDIT: AS pointed to Bryan Chen, You also need to return a reference and non-const, like this:

int& operator [] (int subscript)

Now, looking more in depth at your code, that is not even enough, because you have this method:

ostream & operator << (ostream & output, const IntArray & array) {
    for (int i = array.low(); i <= array.high(); i++) {
        output << array.name << "[" << i << "] = " << array[i] << endl;
    }
    return output;
}

Look that you operator[] needs to work on a non-const IntArray, but in that method your variable "array" is const, so you need to rewrite a bit more of code.

Also, look for the same problem with the rest of the operators, remember: you make a method 'const' only if you don't plan to modify the object from inside that method, and you make a parameter 'const' only if you don't plan to modify that parameter.

Improve this answer
Sign up to request clarification or add additional context in comments.

11 Comments

that's not enough. you should provide overload that return int&. and first const doesn't make any sense
@DWilches I removed the second "const" from the operator[]. However, my ostream and operator= functions now complain, "No viable overloaded operator[] for type 'const IntArray'. Do I need to remove the const declaration from all of these functions?
@BryanChen can you give an example of returning an int&? I thought I was doing this by declaring variable name as a pointer and returning its' value.
Typically you will have two operator[] overloads, one const and one not: int operator[](int index) const; int & operator[](int index);. This provides a read-write non-const implementation, and a read-only const implementation.
@DWilches okay I will try these updated. I originally included "const" because the lab requires that "once an IntArray object has been created, its size cannot be changed". Will removing "const" from operator[] and ostream still allow this to be done?
|
0

Your existing operator does not allow the value to be changed, both because it returns an int by value and because the operator is declared const. You can't assign to a value, only to an object (which includes references, since a reference is just another name for an object).

To accomplish this you need to supplement your existing operator with another, non-const one that returns a reference to the (non-const) int:

int & operator[](int index);

Since this operator would return a reference, you can assign directly to the return value using the familiar a[b] = c syntax you desire to use.

You would not need to change your existing operator, but I would strongly recommend changing the return type from const int to just int -- you are returning by value anyway, so you are handing back a copy. It makes no sense for this to be const, and this may prevent the compiler from eliding copies in the case of more complex data types than int. (It doesn't really make much difference here, but I would avoid getting in the habit of returning both by value and const, since -- assuming the presence of a copy constructor -- the const qualifier can be removed anyway by simply copying the value again. Returning const copies usually provides no benefits while having several drawbacks.)

Improve this answer

Comments

0

Since you also asked to point out your mistakes, I would like to comment on two things you should/could have done to make the code more simple:

First, the assignment operator could have been written like this:

IntArray& operator=(IntArray rhs)
{
   std::swap(rhs.a, a);
   std::swap(rhs.b, b);
   std::swap(rhs.size, size);
   std::swap(rhs.num, num);
   std::swap(rhs.name, name);
   return *this;
}

This works, since you have a copy constructor and destructor already defined for IntArray, and they hopefully work correctly. All the assignment operator is doing is creating a temporary object and swapping out its internals with the internals of the current object. Then the temporary dies with the "old data", while the new data is safely in the current object. This is called the copy/swap idiom.

Also note that a reference is returned that is non-const.

If you pass a const reference instead of an object, then the assignment operator is responsible for the initial copy made. 

IntArray& operator=(const IntArray& orig)
{
   IntArray rhs(orig);
   std::swap(rhs.a, a);
   std::swap(rhs.b, b);
   std::swap(rhs.size, size);
   std::swap(rhs.num, num);
   std::swap(rhs.name, name);
   return *this;
}

The former version may be faster, due to allowing the compiler to optimize the copy of the passed value. However the second form of passing a const reference is what is usually done -- note that the temporary object needs to be created inside the function before proceeding.

Second, your operator + can just use operator +=:

IntArray operator+(const IntArray& rhs)
{
   IntArray temp(*this);
   return temp += rhs;
}

All we did was create a temporary equal to the current object. Then we use += to add rhs and return the result. Nice and simple. Note that operator + returns a new IntArray object, not a const IntArray. In addition, operator += should return a reference to the current object, not bool.

To take advantage of this, your operator += should be rewritten thusly:

IntArray& operator+=(const IntArray& rhs)
{
  //..your current code goes here:
  //...
  return *this;
}

Also, your operator += should not be "erroring out" like that. You need to make the class more robust by attempting to add two IntArrays that may not be the same size. If there really is an error throw an exception. Don't return a bool -- remove the return true; and return false; from the function altogether. Always return *this.

Improve this answer

8 Comments

The IntArray& operator=(IntArray rhs) pattern is generally considered poor design, I believe. It prevents the fast-no-op where you attempt to assign a value to itself, for example, and it creates an extra intermediate object where one is not strictly necessary. It will behave correctly but will perform sub-optimally. It also requires the presence of a copy-constructor, which is not always a correct assumption.
@cdhowie - There are persons that claim that for assignment operators and copy/swap, passing the object will allow the compiler to do the copy, thus taking advantage of any optimizations that may be present. Also, I am assuming that the IntArray class has a correct copy constructor, which is what I mentioned in the answer (the copy/swap won't work without a working copy constructor).
From here is where the research has been done: stackoverflow.com/questions/3279543/…
@cdhowie I do have a copy constructor. Given that, would you still advise against the IntArray& operator=(IntArray rhs) pattern?
@cjan92127 - You can use what cdhowie suggested by passing a const reference. However, the code needs to be changed slightly, but not by much. The bottom line is that you have a working copy constructor and destructor, so whatever you choose, the copy/swap will work.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.