C++ Error when creating new array: Expression must have pointer to object type

Question

I am trying to create a new array with positive values only taken from a already created array, and when I am looping through the original array, the index has an error "Expression must have pointer to object type" I tried doing research on the error, and everyone's situation is different when getting this error so I am on my own with this. Here is my code:

int foo::createNewArray() const {
    int newarray[50];
    int oldarray = oldarray[values];
    int size = (sizeof(oldarray));

    for (int i = 0; i > size; i++){
        if (oldarray[i] > 0)
            newarray[i] = oldarray[i];
    }

The "i" above is what has the error. the oldarray[values] is declared in a seperate class file. Here is the small section of the code where it comes from.

        class foo{
        int oldarray[1];
        enum unit {values};

        public:
        int createNewArray() const;
};

int oldarray = oldarray[values] you should not re-declare oldarray. You are not setting oldarray to all values of the other oldarray. You are creating a single integer "oldarray" and assigning it the value of oldarray[values]... Sorry, this explanation is confusing because of your duplicate naming of oldarray vs oldarray... int size = sizeof(oldarray) is likely not safe in this instance since we're not sure whether oldarray is indeed an array or simply an integer, you will likely simply get the size of an integer, not the number of elements in oldarray. — MobA11y, May 20 '13 at 19:19
@user12074577, Apart from the answers so far, `i > size`, `size = (sizeof(oldarray))`, `if (oldarray[i] > 0)`... One could argue that you should use `std::array` for arrays with known sizes as well. — chris, May 20 '13 at 19:21
I added comments to your code, there were so many issues I thought listing them here was less helpful than just commenting line per line. I'm working on an edit that is the "correct" code, for what I think you're trying to do. — MobA11y, May 20 '13 at 19:29

score 1 · Answer 1 · answered May 20 '13 at 19:19

1

Here you shadow oldarray array with a local int variable:

int oldarray = oldarray[values];

From that point on, until end of block, oldarray means an int, and then rest of the code does not make much sense with that.

answered May 20 '13 at 19:19

hyde

60,639
21
115
176

That's my problem, I am trying to iterate through the specific values in the array oldarray[values], but I cant do oldarray[values[i]] in my loop. That is why I threw it in a local variable so I can iterate through it. – user12074577 May 20 '13 at 19:49
@user12074577 That makes no sense. You iterate the index variable, not the values directly. Inside the `[]` is a single index. Not value or a range or anything like that, but a single index. – hyde May 20 '13 at 21:35

Will Custode · Answer 2 · 2013-05-20T20:27:04.653

The problem is because oldArray needs to be an int*, and not just an int. You are currently setting oldarray to the first value in the array, and not pointing it at the root of the array. So something like int* oldArray = newArray will let you iterate through oldArray using the index operator.

class Foo
{
    int* oldArray;
    int size;

public:
    int* CreateNewArray() const
    {
        int* newArray = new int[size];

        int current = 0;
        for( int index = 0; index < size; index++)
        {
            if(oldArray[index] > 0)
            {
                newArray[current] = oldArray[index];
                current++;
            }
        }

        return newArray;
    }
};

I apologize for haphazardly posting this without compiling. Although this solution may be closer to the metal than would be advised, it is still a valid solution to your problem, assuming oldArray and size are set before this method is called.

I think your comment would be more useful if you were to point out something that is wrong rather than just saying it's wrong. I wouldn't have posted it if I believed it were wrong. — Will Custode, May 20 '13 at 20:17
You could have started by *testing* the code – it doesn’t even compile. Fundamentally you are using the wrong type, it’s a pointer *to pointer* to `int`. The rest follows from that. On a higher level, all this trouble could have been avoided by using high-level C++ types instead of relying unnecessarily on low-level memory operations. — Konrad Rudolph, May 20 '13 at 20:22

MobA11y · Answer 3 · 2013-05-20T19:57:11.720

0

Here are some comments line by line of the problems with this code.

class foo{
    int oldarray[1]; //Do you really want an array of size 1?  Why not just an integer?
    enum unit {values};//An enumeration that only enumerates one thing?  Are you sure you don't really want const int VALUES = 0;  I feel like you don't really even want an enum

    public:
    int createNewArray() const; 
};

int foo::createNewArray() const {
    int newarray[50];  //Magic numbers are bad, what if sizeof(oldarray) > 50?
    int oldarray = oldarray[values];  //Re declaring oldarray as a single integer and assigning it oldarray[values] as its value.
    int size = (sizeof(oldarray));  //is this oldarray an integer or an array of integers???

    for (int i = 0; i > size; i++){  //don't you want i < size??? if size > 0, this loop will never get run.
        if (oldarray[i] > 0) //probably grabbing the correct oldarray(Compilers are smart), but not getting expected values because the array version of oldarray wasn't initialized properly.
            newarray[i] = oldarray[i];
    }

I believe what you're attempting to do is the following:

int* foo::createNewArray() const {
    const int SIZE = sizeof(oldarray);
    int *newArray = int[SIZE];
    for(int i = 0; i < SIZE; i++) {
        if(oldarray[i] > 0) {
            newArray[i] = oldarray[i];
        } else {
            newArray[i] = 0;//In most environments this is unnecessary, but it is safer and good style
        }
    }

    return newArray;
}

Note, that even this code will only work if oldarray is within the scope of this code(not great style, passing it in as an argument to createNewArray would be better, but okay) and was instantiated correctly so that sizeof(oldarray) is the size of the array and not the size of an integer, or perhaps an integer pointer, I forget.

edited May 20 '13 at 19:57

answered May 20 '13 at 19:25

MobA11y

18,425
3
49
76

How does this look through the loop checking if the value in the old array is positive or not? – user12074577 May 20 '13 at 19:33
It doesn't, you can add that in, but to me a function named createNewArray, doesn't mean createNewArrayOfAllThePositiveValues, and your code was so sloppy, I wasn't sure that was intended. – MobA11y May 20 '13 at 19:34
It was in your explanation though, so I fixed it :) – MobA11y May 20 '13 at 19:36
The problem though, is when I want to use the old array, I have to include the enum value. That is why it is oldarray[values], because in my code, there are 3 different oldarray[values]. E.G. oldarray[values2], oldarray[values3]. So I want to use specifically oldarray[values]. But when I get to the loop, and iterate though oldarray[] I cant tell it to specifically look at [values], not just oldarray[]. – user12074577 May 20 '13 at 19:44
So what I tried to do above was set a variable to that array, so I can iterate through specifically oldarray[value]. E.G. int old = oldarray[values]; – user12074577 May 20 '13 at 19:47
There are a lot of problems with what you're saying. A: you shouldn't use an enum as an indice into an array. While most compilers will assign enum values starting with zero, this is not a requirement. You should never count on, or use enums, in situations when you want them to have a specific value. In your case, the index value of 0. This is very poor style, and unless your compiler conforms to enums start with zero(which admitadely most do), will lead you to problems. B: in this case there is absolutely no difference between oldarray[0] and oldarray[value] – MobA11y May 20 '13 at 19:50
Also, you seem to think that you can iterate thorugh oldarray[value]. oldarray[value] is a single integer. If you would like to look at this integer alone, just do that, without any foor loop, though I stand by my statement that looking at oldarray[0], or better yet const int VALUE = 0. Then looking at oldarray[VALUE] is much better style. – MobA11y May 20 '13 at 19:51
@ChrisCM Relying on "enums start from zero" is bad, but [these answers](http://stackoverflow.com/a/900027/1639256) give a reason for `enum { otherValue = 42 };` – Oktalist May 20 '13 at 20:46
@Oktalist: you are correct in picking on my style preferences, shouldn't have used an absolute. But still, anyone competent enough in C++ to consider the true ramifications of const int VALUE vs an enum would not be asking this question :) – MobA11y May 20 '13 at 20:58

C++ Error when creating new array: Expression must have pointer to object type

3 Answers3