Create an object only if there is a condition, otherwise returns nullptr

Question

I want to create an object only if some conditions apply, otherwise retun nullptr. Here is how I would do it in Delphi (2009+):

function GetGen(n : integer) : Generics.Collections.TList<Integer>; var i : integer; begin result := nil; if n > 0 then begin result := Generics.Collections.TList<Integer>.Create; for i := 0 to n - 1 do result.Add(i); end; end; procedure TestGenList(n : integer); var aInt : integer; aGen : Generics.Collections.TList<Integer>; begin aGen := GetGen(n); if aGen = nil then begin WriteLn('No generic created!'); Exit; end; WriteLn(Format('Size: %d', [aGen.Count])); for aInt in aGen do Write(Format('%d ', [aInt])); aGen.Free; //will clear integers end; procedure TestGen begin TestGenList(0); Readln; TestGenList(5); Readln; end. 

Here is how I could do it in C ++:

 unique_ptr<vector<int>> GetUniquePrtVec(int n){ if (n < 1) return(nullptr); //create only if correct input is given unique_ptr<vector<int>> result (new vector<int>); for (int i = 0 ; i != n; i++){ result->push_back(i); } return(move(result)); } void TestPtrVec(int n){ unique_ptr<vector<int>> vec = GetUniquePrtVec(n); if (vec == nullptr){ cout << "No vector created" << endl; return; } cout << endl << vec->size() << endl; for_each(vec->begin(), vec->end(), [](int n){cout << n << " " << endl;}); vec->clear(); //clear vector vec.reset(nullptr); } void testVec3(){ TestPtrVec(0); TestPtrVec(5); } 

My question is about the correct idiom. Guys, experienced C ++ programmers (since I'm new, just learning the language), do it like this? If not, how will you do it?

Thanks.

Answer 1

IMHO, the best way for your example is to simply return the value of std::vector by value and simply return empty if the input is invalid.

 std::vector<int> get_vec(int n){ std::vector<int> ret; for(unsigned i=0; i < n; ++i) ret.push_back(i); return ret; // will be empty for (n < 1) // and will be moved if (n >= 1) } 

One thing you need to learn: you do not need to explicitly specify std::move if you are returning a local variable. Just go back by value. If copying is possible, it will do it (RVO / NRVO). If for some reason this does not help, first try moving it before copying it. Note, however, that the local variable element will not be moved automatically, aka

 struct object{ std::vector<int> member; }; std::vector<int> foo(){ object o; // ... return o.member; // no move, no copy elision, plain old copy } 

Now your second function can also be improved and reduced:

 void try_vec(int n){ auto vec = get_vec(n); // will elide copy or simply move for(auto& x : vec) // will not loop if vector is empty std::cout << x << ' '; // why space and newline? std::cout << "\n"; // don't use std::endl, it also flushes the stream } 

And from your original function:

 vec->clear(); //clear vector vec.reset(nullptr); 

It is not required that all reason for smart pointers and resource management containers. They will destroy what belongs to them when they go out of scope.

Answer 2

I personally think that having a pointer to a vector is a little necessary, it looks as if you can simply return an empty vector or even throw an incorrect argument error. All null returns a bit of a hack, and now you need to manage some memory because of this.

I personally would rather see

 std::vector<int> get_vec(int n){ std::vector<int> result; if(n < 1) return result; result.reserve(n); for (int i = 0 ; i != n; i++){ result.push_back(i); } return result; } 

or

 std::vector<int> get_vec(int n){ if(n < 1) throw std::invalid_argument("n must be greater than 1"); std::vector<int> result; result.reserve(n); for (int i = 0 ; i != n; i++){ result.push_back(i); } return result; } void test(int n){ try{ std::vector<int> vec = get_vec(n); catch(const std::exception& e) { std::cerr << "No vector created: " << e.what() << std::endl; return; } //etc. . . 

Answer 3

It seems like you need something like boost::optional . Here is an example of its use:

 optional<char> get_async_input() { if ( !queue.empty() ) return optional<char>(queue.top()); else return optional<char>(); // uninitialized } void receive_async_message() { optional<char> rcv ; // The safe boolean conversion from 'rcv' is used here. while ( (rcv = get_async_input()) && !timeout() ) output(*rcv); } 

See the additional documentation for more information.

Answer 4

Using exceptions or erasing styles that returns NULL is a way to do something, not C ++.

You also use the semantics of move, but you do not return the value of r, it will not work like that.

Answer 5

I am a little weak with this syntax, but I think it looks good to me. Although, why not just use pointers with the usual c + syntax?

 vector<int> GetUniquePrtVec(int n) { if (n < 1) return null; vector<int>* result = new vector<int>; for (int i = 0 ; i != n; i++){ result->push_back(i); } return (result); } 

Although Ive never used a vector pointer. Usually, when I create a vector, I pass it to the function by reference, for example:

 vector<int> myVec; bool bSuccess = PopulateVec(n, myVec); vector<int>* PopulateVec(int inNum, vector<int>& inVec) { if (inNum< 1) return false; for (int i = 0 ; i != inNum; i++) { inVec->push_back(i); } // inVec is "returned" by reference return true }