I know that the answer will be obvious, probably not, but I'm just not completely sure.
For example somewhere it code in class/struct a have vector with objects in my case is Asset:
class Asset {
int ID = - 1;
}
std::vector<Asset> GAssets;
And then i also have function that retrive asset by its ID. If asset is found i can return it by reference to modify its actually content inside somewhere in my code, but if asset not found i need to return invalid asset with ID -1. To do this i basically can return new stack object, but here my problem, i think how this would work? It return my asset that it found is right but in case with invalid asset and new stack object, it removes it form stack or return it?
Asset& GetAssetByID(int ID) {
for(Asset& asset : GAssets) {
if(asset.ID == ID)
return asset;
}
return Asset(-1); // How this would returns?
}
NOTE: I'll tell you right away I could use pointers, but I don't need them because there are a lot of assets and I don't know when I will need to delete them. It's the same with smart pointers.
CodePudding user response:
Simply return a pointer instead of a reference, and then you can return nullptr
when no asset is found.
Returning a pointer from GetAssetByID does not make any difference to how you delete the assets or not.
CodePudding user response:
Originally my idea was to propose returning a statically allocated object, like this:
const Asset& GetAssetByID(int ID) {
...
static const Asset not_found{ -1 };
return not_found;
}
However it seems to contradict signature of your function, because you apparently need a mutable reference. You may omit const
qualifier here for both return type and the not_found
variable, but since your ID
member is also mutable, it's not really a good design (the client code may change the member variables of the returned value by mistake and never find it was an invalid asset in the first place, let alone this will break the logic for all future calls). So you should consider changing ID
to const int
if possible:
class Asset {
const int ID = - 1;
...
}
Asset& GetAssetByID(int ID) {
...
static Asset not_found{ -1 };
return not_found;
}
If it's not an option, please take a look at the next proposal below.
A way more consistent option would be throwing an exception when no asset is found:
Asset& GetAssetByID(int ID) {
...
std::ostringstream os;
os << "Could not find an asset with ID: " << ID;
throw std::invalid_argument{ os.str() };
}
The client code can use it like this:
int main() {
try {
auto& asset = GetAssetByID(20);
} catch (std::invalid_argument error) {
std::cerr << error.what() << std::endl;
}
return 0;
}
CodePudding user response:
This code contains a dangling reference:
Asset& GetAssetByID(int ID) {
for(Asset& asset : GAssets) {
if(asset.ID == ID)
return asset;
}
return Asset(-1); // How this would returns?
}
That is, you return a reference to an object on the stack that gets deallocated, which would be undefined behavior, so who knows what this returns? :)
I would suggest a different approach to your problem: instead of storing the asset's ID inside the Asset
class, you can address your std::vector
of Assets
by the ID. In the client code you could then write:
std::vector<Asset> assets;
//...
try {
auto& a = assets.at(id);
// do something with the asset
} catch (std::exception const& ex) {
// asset was not found
}