1

I have a function

double* get_row(double *the_array, int row_num, int col_size)    

where the_array is a 2D array, row_num is the row number on the 2D array where i want to copy the values from, and col_size is the column size of the 2D array.

Function Implementation

double* get_row(double *the_array, int row_num, int col_size) {

    double* row = new double[col_size];

    for (int i = 0; i < col_size; i++) {

        *row = *(*(the_array + row_num) + i);
        row++;
    }

    delete[] row;

    return row;
}

im trying to copy values from row row_num of the 2D array the_array into the 1D array row but the compiler gives me an error on the line *row = *(*(the_array + row_num) + i); which i used to copy elements from one array to the other, what is the correct way to complete such task.

BTW the error im getting from visual studio is : operand of '*' must be a pointer.

thank you for your help.

8
  • 3
    double *the_array is not a 2d array Commented Sep 11, 2018 at 18:26
  • 2
    delete[] row; return row; is an clear and immediate red flag. You can't use something you've just deleted. Commented Sep 11, 2018 at 18:26
  • im sorry but those are the directions given to me from the assignemnt i must pass a 2D array into the function, also i cannot make any changes to the function declaration. Commented Sep 11, 2018 at 18:31
  • 1
    Sigh. Why is C++ still being taught as "C with classes"? This should be using some combination of std::array, std::vector, std::copy, std::unique_ptr, std::shared_ptr, range based for loops, etc. Not manual memory management, C-style arrays, raw pointers. C++ should be taught as C++17 first, then the ugly legacy stuff that we all try to avoid, later (if needed at all). Code like this makes me sad. Commented Sep 11, 2018 at 18:43
  • 1
    @Drt No it is not. It can decay to a pointer in many cases but it is itself not a pointer. decltype(array_name) will never yield a pointer. Commented Sep 11, 2018 at 18:46

4 Answers 4

1

I'll assume "col_size" is actually "number of columns" in order to solve this problem. And that the data is arranged in row-major order.

Then the start of row N is located at the_array + row_num*col_size.

I'd start with this helper function:

inline double* view_row_n( double* array, int row, int num_cols ) {
  return array+row*num_cols;
}

then I'd write a copy-buffer:

 std::unique_ptr<double[]> copy_buffer( double const* in, int count ) {
   double* retval = new double[count];
   std::memcpy( retval, in, sizeof(double)*count );
   return std::unique_ptr<double[]>( retval );
 }

now I'd use them together:

double* get_row( double* the_array, int row_num, int col_size) {
  auto retval = copy_buffer( view_row_n( the_array, row_num, col_size), col_size );
  return retval.release();
}

and be done with it.

I'd also change:

double* get_row( double const* the_array, int row_num, int col_size) {

and even:

std::unique_ptr<double[]> get_row( double const* the_array, int row_num, int col_size) {

but that is beyond the scope of your problem.

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

6 Comments

You used a smart pointer. Now some poor marker's head's going to explode. Either you are a fiend or a big fan of Scanners.
@user4581301 Smart pointers are great but when the OP is forced to write code like this there isn't much you can do.
I've downvoted this answer. It's perfectly right, professionally right. But it doesn't allow the user to understand pointer arithmetic with arrays, which IMHO is the goal of the assigment.
@Ripi2 going to disagree with that. It simply moved the pointer arithmetic. It's all still at array+row*num_cols
@user4581301 I try to solve problems as right as I can within constraints. The OP is free to extract the part of my answer that doesn't include smart pointers, but barring a hard requirement I'm not going to have a function returning an owned buffer without using an owning data type. I'm not smart enough to do that right.
|
1

If get_row doesn't require you to allocate new memory back to the caller, then the solution would simply be

double* get_row(double *the_array, int row_num, int col_size) 
{
    return the_array + (row_num * col_size);
}

If the caller is expected to allocate new memory to the returned row, then

double* get_row(double *the_array, int row_num, int col_size) 
{
    double* row = new double[col_size];
    for(int i = 0; i < col_size; i++)
        row[i] = *(the_array + (row_num * col_size) + i);
    return row;
}

would work.

In the first solution, the idea is to use row_num * col_size to get to the correct row and simply return that address location as the result. In the second solution, we first declare a new row. And then use i to index each individual element in that row and copy each element over from the_array at that row indexed by the_array + (row_num * col_size).

Comments

0

This is not modern-day C++, and obviously you should not be deleting your return value.

im trying to copy values from row row_num of the 2D array the_array into the 1D array

First off, the_array in this context, being defined as a raw poiner and used for pointer arithmetic, is a 1D representation of a 2D array -- better be accurate in our description.

the error im getting [...] is : operand of '*' must be a pointer

Ok, so this line:

*row = *(*(the_array + row_num) + i);

is kind of a mess. I think I see where you were going with it, but look, you're dereferencing (using * to the left of an expression) twice for no reason, which BTW causes the error. Remember we said the_array is a raw pointer, being treated as a 1D array. Hence, you can use this type of pointer arithmetic, let's say inside the parenthesis, and then dereference it just once. Your expression above takes the resultant address of (the_array + row_num), dereferences it to get the double within this cell of the array, and then adds i to the double value itself and then tries to dereference this sum of a double and int i -- which is a temporary variable of type double, not much of a pointer. This here is probably the line you were aiming for:

*row = *(the_array + row_num * col_size + i);

Because the 2D data is spread contagiously in memory, row by row, consecutively, you need to multiply the row index like this by the row size (which is also the column count, and I take it that col_size is actually that, otherwise you have no way of traversing between rows) to "skip between the rows" and finally add the current cell index i to get to the specific cell inside the row. Then, like we said, dereferene the whole thing.

Beyond the compilation issue and address calculation, you should at least keep address of row before incrementing it in the loop so you could be returning it and not the pointer past-the-end of the row. In keeping as much as possible with your original function, you can do it using a second pointer like this:

double* get_row(double *the_array, int row_num, int col_size) {

    double* row = new double[col_size];
    double* rowPtr = row;

    for (int i = 0; i < col_size; i++) {
        *rowPtr = *(the_array + row_num * col_size + i);
        rowPtr++;
    }

    return row;
}

I'm trying to keep this as closest as possible to your version, so you could see the difference from what you did. Many things are better done differently, for example why not use the [] operator in the first place like this row[i] = *(the_array + row_num * col_size + i); and then get rid of the second pointer altogether? No real need for doing ++ there as a second line in the loop if you're iterating over i as it is. Another example is did you have to allocate or could you just return a pointer to the existing array but in the right place?

Too important to not mention here is that in almost any scenario you really shouldn't be returning a raw pointer like this, as it could too easily lead to unclear intent in the code, as to who owns the allocation (and is responsible to delete it eventually) and raises the issue of how to prevent memory-leak in case of an exception? The preferred solution is to be using smart pointers, such as std::unique_ptr, an instance of which wraps the raw pointer and makes it so you're going to have to be very explicit if you'll want to just throw that piece of memory around in an unsafe way.

5 Comments

You need to save the original value of row in another variable, otherwise it's returning a pointer to the end of the row, not the beginning.
"This is barely C++" I know what you mean, but I disagree with the wording. This could be perfeclty C++ (apart from the logic errors), its just no c++ that one should write nowadays
@Barmar Thanks I was adding it while you were writing the comment I think. Was building it incrementally. Thanks anyways
I apoligize for the error where I deleted row before returning it, im just beginning to learn how to program, not for school or anything im just curious about the field i managed to teach myself basics such as functions, clases, loops, etc. but im having a Little bit of trouble understanding pointers and array relationships so I asked.
@JosueM No need to apologize friend. I am just trying to help, and being as informative as I can during. Let me know if you have further questions about this.
0
double* get_row(double *the_array, int row_num, int col_size) {
    double* row = new double[col_size];

    for (auto i = 0; i < col_size; ++i) {
        row[i] = the_array[row_num * col_size + i];
    }

    // If the return isn't assigned, the program will leak!
    return row;
}

void row_test() {
    double two_d[15];

    const auto row = get_row(two_d, 2, 5);

    // ...
    // Be sure to delete row afterwards
    delete[] row;
}

1 Comment

"// Be sure to delete row afterwards" - No. I object. row should not need manual cleanup. It should be a std::array or std::vector or a std::unique_ptr to some memory. (created with std::make_unique). A naked delete or delete[] is a code smell. Don't do it. We have better ways these days.

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.