initializing a c++ vector with values

Question

I need to init my vector with a class I have created holding x,y double values. is there a better way to init it?

std::vector<CentroidXY> centroids;
CentroidXY temp = CentroidXY(1504.907526094 , 1345.27375938);
centroids.push_back(temp);
temp = CentroidXY(1843.890860395045,1694.073652494);
centroids.push_back(temp);
temp = CentroidXY(1852.11101474414,1354.88360797918);
centroids.push_back(temp);

here is CentroidXY.cpp

#include "CentroidXY.h"

CentroidXY::CentroidXY(double X, double Y)
{
    m_X = X;
    m_Y = Y;
}
CentroidXY::CentroidXY(void)
{
}

CentroidXY::~CentroidXY(void)
{
}

here is CentroidXY.h

#pragma once
class CentroidXY
{
public:
    CentroidXY(double X, double Y);
    CentroidXY(void);
    ~CentroidXY(void);

    double m_X;
    double m_Y;
};

Thanks for accepting my answer. However I got a -1, so if you find any bug (if the -1 is for this reason) please let me know, so that the future readers won't face it! — gsamaras, Mar 30 '15 at 12:18

score 3 · Accepted Answer · edited May 23 '17 at 11:50

3

I would use an initialization list, so change this:

CentroidXY::CentroidXY(double X, double Y)
{
    m_X = X;
    m_Y = Y;
}

to this:

CentroidXY::CentroidXY(double X, double Y) : m_X(X), m_Y(Y)
{ }

Source

Moreover, you could change this (since you don't need temp):

CentroidXY temp = CentroidXY(1504.907526094 , 1345.27375938);
centroids.push_back(temp);

to this:

centroids.push_back(CentroidXY(1504.907526094 , 1345.27375938));

or (which is equivalent to calling push_back):

centroids.emplace_back(CentroidXY(1504.907526094 , 1345.27375938));

or even better:

centroids.emplace_back(1504.907526094 , 1345.27375938);

That way the code is cleaner.

Interesting link: push_back vs emplace_back

edited May 23 '17 at 11:50

Community

1
1

answered Mar 30 '15 at 09:25

gsamaras

71,951
46
188
305

2

Also, `emplace_back` could be used. – juanchopanza Mar 30 '15 at 09:28
2

I guess you should use `centroids.emplace_back(1504.907526094 , 1345.27375938);` – Ruggero Turra Mar 30 '15 at 09:35
1

What @RuggeroTurra said. Otherwise you're emplacing using one of the copy constructors, which is the same as calling `push_back` in the first place :-) – juanchopanza Mar 30 '15 at 09:36
Thank you @RuggeroTurra, edited! Agreed juanchopanza! – gsamaras Mar 30 '15 at 09:36
1

I got a -1 and I would like to know if there is a bug, so that I can edit! Think of the future readers! – gsamaras Mar 30 '15 at 12:17

Robinson · Answer 2 · 2015-03-30T11:18:53.170

3

Could use:

auto centroids = std::vector<CentroidXY>() = {
    { 1504.907526094, 1345.27375938 },
    { 1852.11101474414, 1354.88360797918 }
};

or if you can't stand Almost Always Auto (AAA), use:

std::vector<CentroidXY> centroids = {
    { 1504.907526094, 1345.27375938 },
    { 1852.11101474414, 1354.88360797918 }
};

edited Mar 30 '15 at 11:18

answered Mar 30 '15 at 09:40

Robinson

9,666
16
71
115

Very nice answer, +1! :) Pity I did not include it to my answer! – gsamaras Mar 30 '15 at 09:41
Yes. The Standard is The Standard I guess. – Robinson Mar 30 '15 at 09:43
1

I mentioned it because the OP tagged the question `C++` and not `C++11` – CinCout Mar 30 '15 at 09:44
1

@GargAnkit I'm not using c++11 so I can't use this answer. – Gilad Mar 30 '15 at 10:13
The answer you ticked includes emplace_back, which is C++11. – Robinson Mar 30 '15 at 10:19
Why do you propose a complicated code like `auto centroids = std::vector() = {{ 1504.907526094, 1345.27375938 },...`? Wouldn't the following one be simpler? `std::vector centroids = {{ 1504.907526094, 1345.27375938 }, ...};` – Mr.C64 Mar 30 '15 at 11:07
I use "auto" for all declarations except class members. It's not "complicated". It's simple C++. And Garg you posted a reply that included "new" with push_back, so I would be a little more circumspect if I were you. See http://herbsutter.com/2013/08/12/gotw-94-solution-aaa-style-almost-always-auto/. – Robinson Mar 30 '15 at 11:12
@Robinson I am not saying that you are wrong. I am only saying that the solution you gave isn't useful in this context. That's it. – CinCout Mar 30 '15 at 13:02
If my answer is wrong because it's up to date with the current C++ standard, so is the answer the OP accepted as it uses a C++11 function (emplace). – Robinson Mar 30 '15 at 13:09
Well, that's not my call to take! – CinCout Mar 30 '15 at 13:20

CinCout · Answer 3 · 2015-03-30T09:39:56.850

1

How about this:

std::vector<CentroidXY> centroids;
centroids.push_back( CentroidXY( 1504.907526094 , 1345.27375938 ) );
centroids.push_back( CentroidXY( 1843.890860395045, 1694.073652494 ) );
centroids.push_back( CentroidXY( 1852.11101474414, 1354.88360797918 ) );

edited Mar 30 '15 at 09:39

answered Mar 30 '15 at 09:26

CinCout

9,486
12
49
67

I am not the guy who gave you the -1, but `new` should probably be the reason. See my answer Garg, you were close. :) – gsamaras Mar 30 '15 at 09:33
Who cares about the downvote! Can you still throw some light on why `new` isn't correct here? – CinCout Mar 30 '15 at 09:34
4

Well, there isn't any need for calling `new`, and it will most likely result in a compiler error, or a memory leak. – juanchopanza Mar 30 '15 at 09:36
1

Also if calling new, the pointer should be `delete`d when it is no longer required in order to reduce the possibility of a memory leak. – AStopher Mar 30 '15 at 09:42

initializing a c++ vector with values

3 Answers3