0

In the following code when I edit drop_location.materials it doesn't edit the drop_location that already exists in prod_config. Where is the found drop_location being copied, and how do I avoid the copy operate directly on the field in prod_config?

  auto prod_config = getProdConfig(nh);
  models::dropLocation_t drop_location;
  getDropLocationByName(prod_config, query_drop_location_name, drop_location);
  drop_location.materials = materials;
void getDropLocationByName(models::prod_t& prod_config, std::string query_drop_location_name, models::dropLocation_t& drop_location_out)
{
  auto& drop_locations = prod_config.dropLocations;
  auto it = std::find_if(
      drop_locations.begin(), drop_locations.end(), [&query_drop_location_name](auto drop_location) {
        return drop_location.name == query_drop_location_name;
      });
  if (it == drop_locations.end()) {
    std::stringstream ss;
    ss << "Drop location not found: " << query_drop_location_name;
    throw HTTP404Exception(ss.str());
  }
  drop_location_out = *it;
}
  • 3
    `T x = y;` makes `x` a copy of `y`. Use `T&` to make `x` be an alias for `y` – M.M Aug 26 '21 at 02:12
  • 1
    @Lala5th preferably use `auto&` (DRY principle) – M.M Aug 26 '21 at 02:15
  • 1
    You can also `delete` the copy constructor https://stackoverflow.com/questions/6077143/disable-copy-constructor and the compiler will tell you via an error where the copy is happening. – Matt Aug 26 '21 at 02:21
  • Which line is doing: `T x = y;`? – Alexis Winters Aug 26 '21 at 02:26
  • Try this: `auto& drop_locations = prod_config.dropLocations;` and we also need to change the functiom `getDropLocationByName` to return a reference type. – prehistoricpenguin Aug 26 '21 at 02:28
  • Okay, yeah that was definitely an issue, but I made the changes (reflected in the edit above), and still seeing the same behavior. – Alexis Winters Aug 26 '21 at 02:34
  • This is caused by drop_location is not a reference, so another copy happens, try to change the function into `models::dropLocation_t& getDropLocationByName(models::prod_t& prod_config, std::string query_drop_location_name)` – prehistoricpenguin Aug 26 '21 at 02:37
  • That worked. It seems it isn't possible with the outvar then? – Alexis Winters Aug 26 '21 at 02:39

2 Answers2

1

It's not just that it's being "copied", it's just that it's a completely independent object.

models::dropLocation_t drop_location;

This creates a completely independent, separate object, called drop_location.

Anything that's changed in this object, any member of this class instance that get modified, affects this object only. It will never have any effect on any other object that might exist somewhere.

 getDropLocationByName(prod_config, query_drop_location_name, drop_location);
 drop_location.materials = materials;

The effect of the function call appears to be to find some particular models::dropLocation_t instance that exists in prod_config and then copy it into drop_location. Ok, so something from prod_config was copied into drop_location. Great. Then drop_location.materials get modified. So far so good. The end result: some object from prod_config gets copied into drop_location, and then something else in drop_location called "material` gets modified.

And that's the end of the story. You seem to expect that just because that something was originally copied from prod_config into drop_location then this must mean that this modification affects the original object. That's approximately how objects might behave in Java or C#, but C++ is neither one of those two languages. Reduced into the most simple form, this is nothing more than:

 int c=0;

 int b=c;

 b=3;

c is still 0, only b gets changed to 3. Just because b's value originally came from c doesn't mean that this also changes c to 3. The same thing applies to the shown code, as well. drop_location.materials=materials;, where drop_location is an object, only affects an object called drop_location, and no other object. The End. It does not affect anything else, and has no effect on whichever object the rest of drop_location may or may not have been copied from. drop_location is an independent object. It's not that its copied (and it was), it's just all objects in C++ are, well, discrete objects. In other languages, like Java, all objects are really pointers, so copying a so-called "object" from one place to another, and then modifying the "copied" object also ends up modifying the original "object", but that's not how objects work in C++.

Sam Varshavchik
  • 114,536
  • 5
  • 94
  • 148
1

To change existed values, we need to access then by reference or pointer. So a fixed code might like:

auto prod_config = getProdConfig(nh);
try {
  auto& drop_location = getDropLocationByName(
      prod_config, query_drop_location_name, drop_location);
  drop_location.materials = materials;
} catch (...) {
}

models::dropLocation_t& getDropLocationByName(
    models::prod_t& prod_config, std::string query_drop_location_name) {
  auto& drop_locations = prod_config.dropLocations;
  auto it =
      std::find_if(drop_locations.begin(), drop_locations.end(),
                   [&query_drop_location_name](auto drop_location) {
                     return drop_location.name == query_drop_location_name;
                   });
  if (it == drop_locations.end()) {
    std::stringstream ss;
    ss << "Drop location not found: " << query_drop_location_name;
    throw HTTP404Exception(ss.str());
  }
  return *it;
}

Or via pointer:

auto prod_config = getProdConfig(nh);
try {
  auto* drop_location = getDropLocationByName(
      prod_config, query_drop_location_name, drop_location);
  drop_location->materials = materials;
} catch (...) {
}
models::dropLocation_t* getDropLocationByName(
    models::prod_t& prod_config, std::string query_drop_location_name) {
  auto& drop_locations = prod_config.dropLocations;
  auto it =
      std::find_if(drop_locations.begin(), drop_locations.end(),
                   [&query_drop_location_name](auto drop_location) {
                     return drop_location.name == query_drop_location_name;
                   });
  if (it == drop_locations.end()) {
    std::stringstream ss;
    ss << "Drop location not found: " << query_drop_location_name;
    throw HTTP404Exception(ss.str());
  }
  return &*it;
}

prehistoricpenguin
  • 6,130
  • 3
  • 25
  • 42