How to allocate a vector of N(non-const variable) pairs & initialize them?

Question

I'm making a function that takes four integers(line coordinates) then returns a pointer to a vector of pairs(pixel coordinates), I'm having trouble allocating the vector and initializing the pairs to zero.

the compiler is not accepting an expression in vector allocation: expected a type, got '(X1 - Xo)'

If someone could explain to me how new works, does it create actually the vector & there are pairs object in it uninitialized , or does it just reserve the mentioned object's size?

my function code is:

vector<pair<int, int>>* draw_line_DDM(int Xo, int Yo, int X1, int Y1)
{
    double m;

    if((X1-Xo) == 0 && (Y1-Yo) == 0)
    {
        vector<pair<int, int>> *point = new vector<pair<int, int>>(1, make_pair(X1, Xo));
        //return a vector of one pair only

        return(point);  
    }


    if((X1-Xo) == 0 && (Y1-Yo) != 0)
    {
        m = 1000000000.0;
    }


    if((X1-Xo) != 0 && ((Y1-Yo) == 0 || (Y1-Yo) != 0))
    {
        double m = (Y1-Yo)/(X1-Xo);
    }


    if(abs(m)<=1)
    {
        vector<pair<int, int>> *pixels_x = new vector<pair<int, int>>(sizeof(vector<(X1-Xo),pair<0, 0>>));

        double y = Yo;

        for(int counter_x = Xo; counter_x <= X1 ; counter_x++)
        {
            pixels_x->at(counter_x).first = counter_x;
            y += m;
            pixels_x->at(counter_x).second = round(y);

        }
        //create a vector of x pairs

        return(pixels_x);
    }

    if(abs(m)>1)
    {
        double x = Xo;
        vector<pair<int, int>> *pixels_y = new vector<pair<int, int>>(sizeof(vector<(Y1-Yo),pair<0, 0>>));
        for(int counter_y = Yo; counter_y <= Y1 ; counter_y++)
        {
            pixels_y->at(counter_y).second = counter_y;
            x += (1/m);
            pixels_y->at(counter_y).first = round(x);   
        }

        return(pixels_y);
    }

}

`vector>(sizeof(vector<(X1-Xo),pair<0, 0>>));` why do you think you need `sizeof` here? — 463035818_is_not_an_ai, Nov 07 '18 at 18:01
and, clearly `pair<0, 0>` is wrong. Pretty sure you meant `std::make_pair(0,0)`. Also, `double m = (Y1 - Yo) / (X1 - Xo);` inside that if-condition is pointless unless you lose the `double` part. It hides the outer `m` and makes the calculated value worthless. — WhozCraig, Nov 07 '18 at 18:02
seems like you are confusing types with values. `vector>` is the type and the size of a vector is not part of its type. To the constructor you need to pass the number of elements you want, not the `sizeof` something — 463035818_is_not_an_ai, Nov 07 '18 at 18:03
i'd write an answer, but imho there should be no pointer in your code which makes the question moot... — 463035818_is_not_an_ai, Nov 07 '18 at 18:05
Sidenote: Automatically allocate the `vector`. Because `vector`'s job is to spare the programmer from having to deal with manual memory management, you almost never want to pick up the task of memory management again by dynamically allocating a `vector`. Compilers are amazingly good at eliminating copying when you pass them around by value and for the cases where you need to pass by reference, use a reference rather than a pointer. — user4581301, Nov 07 '18 at 18:06
It is a terrible idea to return a raw pointer to dynamically allocated object, especially vector — Slava, Nov 07 '18 at 18:06
actually I suggest you to start from scratch. In all honesty, this code is a mess. Dont write that much without making sure inbetween that you are on the right track. Write one line, compile, write the next line. And simply dont return a pointer but a vector — 463035818_is_not_an_ai, Nov 07 '18 at 18:13
@user463035818 my question was specific, "how the allocation & initialization of a vector of pairs is done?", not if there is a point to this pointer. but fair enough. — topcat, Nov 07 '18 at 18:17
`new` (the short version) allocates storage for the object (or fails and throws `std::bad_alloc`) and then calls the appropriate constructor, if any. The long version: https://en.cppreference.com/w/cpp/language/new — user4581301, Nov 07 '18 at 18:21
Handy additional reading: [Why should C++ programmers minimize use of 'new'?](https://stackoverflow.com/questions/6500313/why-should-c-programmers-minimize-use-of-new) — user4581301, Nov 07 '18 at 18:22
yes the question is specific and clear, I just think its not helping to blindy provide the answer without pointing out what I think is a wrong premise or misconception. I dont want to be responsible for a single raw owning pointer out there :P — 463035818_is_not_an_ai, Nov 07 '18 at 18:27
the point is that you should never (almost never) `new` a vector, there is no need. Just treat them like `int` , dont over think them (many people have done a whole lot of thinking for you under the hood) — pm100, Nov 07 '18 at 18:36

463035818_is_not_an_ai · Answer 1 · 2018-11-07T19:48:22.173

There are many problems in your code, not sure if a can cover all. The most important issue is that there is no reason to return a raw pointer. Return the vector instead. To initialize the vector you just need to call the constructor that takes the number of elements as first parameter (and note that std::pair has a default constructor):

#include <utility>
#include <vector>

using pair_vector = std::vector<std::pair<int,int>>;
pair_vector foo() {
    pair_vector result(20);
    return result;
}

To return a vector of a single pair you could write

pair_vector get_vector_with_single_pair() {
    return { {1 ,1} };
}

And if you want to populate the vector in a loop you could do

pair_vector get_vector(int n) {
    pair_vector result;
    result.reserve( n );
    for (int i=0;i<n;++i){
        result.emplace_back( i,i );
    }
    return result;
}

(un)related problems

Here

if((X1-Xo) != 0 && ((Y1-Yo) == 0 || (Y1-Yo) != 0))
{
    double m = (Y1-Yo)/(X1-Xo);
}

m is local to the body of the if. You most likely want to use the m declared on the functions scope instead. Once you fix that, the next thing to discover is integer arithmetics. As all the variables involved are ints the (Y1-Yo)/(X1-Xo) part will use integer arithmetics, where for example 5 / 6 == 0. I suppose this is not what you want, so you need to cast to double before you get the result, eg

double dy = Y1-Y0;
double dx = X1-X0;
m = dy / dx;

Moreover, in some cases, you want to return a vector containing Y1-Y0 elements. Note that this is not consistent with your loop counters:

for(int counter_y = Yo; counter_y <= Y1 ; counter_y++)

this loop has Y1-Y0+1 iterations, so you will be off by one. You can fix it either way, but I strongly suggest you to use half open intervals, as this is what all standard algorithms are using. Meaning, change your loop to

for(int counter_y = Y0; counter_y < Y1 ; ++counter_y)
                               // ^----------------------- Y1 is excluded

and adjust the parameters you pass accordingly.

Last but not least, choose better variable names. Your code would be much easier to read if for example m would be called slope. For the others I have no clue what they are.

Isn't returning arrays forbidden in c++ similar to C? why then return a vector? what is the difference in time between returning an entire vector vs a vector pointer? — topcat, Nov 07 '18 at 19:22
@topcat you cannot return a c-style array in either language, a `std::vector` has no such problems — 463035818_is_not_an_ai, Nov 07 '18 at 19:25
@topcat read [this](https://stackoverflow.com/questions/12953127/what-are-copy-elision-and-return-value-optimization). in short: in most cases there is no copy when you return an object from a function. — 463035818_is_not_an_ai, Nov 07 '18 at 19:29
@topcat if `new` and `delete` arent in the same scope you have roughly 200% chance of a memory leak (seriously!). If the function expects the caller to take ownership it should return a smart pointer, but you dont need that, just return an object — 463035818_is_not_an_ai, Nov 07 '18 at 19:43

topcat · Accepted Answer · 2018-11-08T09:11:32.330

I made several mistakes in the code posted in the question:

Firstly in the initialization of vectorOfPairs.
Then in the range of the for loop, trying to access last element crashed program, done better with iterators.
Also in assigning expression of integers to a double(always made the slope = zero).

I agree with @user463035818 regarding memory leaks, passing the de-allocation to the user of the function. but that was not my question.

returning objects leads to copying slowing down program.

Here is the correction of the code in my question:

vector<pair<int, int>>* draw_line_DDM(int Xo, int Yo, int X1, int Y1)
{
    static double m;

    if((X1-Xo) == 0 && (Y1-Yo) == 0)
    {
        vector<pair<int, int>> *point = new vector<pair<int, int>>(1, make_pair(X1, Xo));
        //return a vector of one pair only

        return(point);  
    }


    else if((X1-Xo) == 0 && (Y1-Yo) != 0)
    {
        m = 1000000000.0;
    }


    else
    {
        m = (double)(Y1-Yo)/(X1-Xo);
    }


    if(abs(m)<=1)
    {
        vector<pair<int, int>> *pixels_x = new vector<pair<int, int>>(abs(X1-Xo),make_pair(0, 0));

        double y = (double)Yo;
        double x = (double)Xo;

        //for(int counter_x = 0; counter_x <(X1-Xo) ; counter_x++)
        for(vector<pair<int, int>>::iterator it = pixels_x->begin(); it != pixels_x->end(); ++it)
        {
            it->first = x;
            if(Xo>X1)
                x--;
            if(Xo<X1)
                x++;
            it->second = round(y);
            y += m;

        }
        //create a vector of x pairs

        return(pixels_x);
    }

    if(abs(m)>1)
    {
        double y = (double)Yo;
        double x = (double)Xo;
        vector<pair<int, int>> *pixels_y = new vector<pair<int, int>>(abs(Y1-Yo),make_pair(0, 0));
        //for(int counter_y = 0; counter_y <(Y1-Yo) ; counter_y++)
        for(vector<pair<int, int>>::iterator it = pixels_y->begin(); it != pixels_y->end(); ++it)
        {
            it->second = y;
            if(Yo>Y1)
                y--;
            if(Yo<Y1)
                y++;
            it->first = round(x);   
            x += 1/m;
        }

        return(pixels_y);
    }

};

How to allocate a vector of N(non-const variable) pairs & initialize them?

2 Answers2