-1

Trying to avoid the copy-assignment of object by mentioning as delete to assignment overloading. But still, below code is not showing error for copy assignment. Vehicle class object trying to avoid the assignment operation after creating the new class object.

    class Vehicle {
  private:
    const int _registrationNum;
    const int _yearOfRegistration;
    const int _saleAmount;
    std::string _currentOwnerName;
    OwnersList* _listOfOwners;
  public:
    Vehicle(int regNum,  int year, int saleAmount,std::string& name, std::string& addr)
       :_registrationNum(regNum),
       _yearOfRegistration(year),
       _saleAmount(saleAmount),
       _currentOwnerName(name) {
       _listOfOwners=new OwnersList(name,addr,year);
     }

     Vehicle(const Vehicle&)=delete;
     Vehicle(const Vehicle*)=delete;

     void operator=(const Vehicle&)=delete;
     Vehicle* operator=(const Vehicle*)=delete;

      void printDetails() {
        std::cout<<"\n Registration Number :"<<_registrationNum;
        std::cout<<"\n Year Of Registration:"<<_yearOfRegistration;
        std::cout<<"\n Sale Amount     :"<<_saleAmount;
        std::cout<<"\n Current Owner Name  :"<<_currentOwnerName;
        OwnersList* itr =_listOfOwners;
        while(itr != nullptr) {
           std::cout<<"\n Previous Owner Name: "<<itr->getOwnerName();
           std::cout<<"\n Previous Sale Year : "<<itr->getSaleYear();
           std::cout<<"\n Address        : "<<itr->getAddressOfOwner();
           itr=itr->nextOwner;
        }
     }
};

int main() {
   cout<<"\n +++ Checking for Vehicle Registration +++ \n";
   std::string name="shrikant";
   std::string address="Indi";
   Vehicle* newVehicleObj = new Vehicle(9876,2021,200000,name,address);
   newVehicleObj->printDetails();
   Vehicle* newVehicleObj1 = new Vehicle(987,2022,300000,name,address);
   newVehicleObj1->printDetails();
   newVehicleObj=newVehicleObj1;
   newVehicleObj->printDetails();
   return 0;
}

Please check and correct me.

Shrikant B
  • 25
  • 4
  • `newVehicleObj=newVehicleObj1;` is copying pointer to pointer. This is not involved in any of your overloads. My caffeinated brain can't honestly think of how (or why) to even do this right now. Aside: You're also leaking memory with that assignment. – WhozCraig Aug 26 '21 at 18:14
  • Looks to me like you're assigning pointers to objects, not the objects themselves. You're copying addresses. To get the behaviour you're expecting, `*newVehicleObj = *newVehicleObj1;` – user4581301 Aug 26 '21 at 18:15
  • while reading about copy-assignment and avoiding the '=' opeations, mentioned as 'delete'. But in my case, I am trying to assign two class Objects. But still it should fail. Is there any approach to avoid this ? – Shrikant B Aug 26 '21 at 18:18
  • 5
    *"I am trying to assign two class Objects"* - No, you're not. You're trying to assign one pointer value (an address held by `newVehicleObj1`) to another pointer lvalue (`newVehicleObj` , which also holds an address). Neither of your overloads cover that (nor can they). Your overloads cover assignment of `Vehicle& = const Vehicle&` and `Vehicle& = const Vehicle*`. Your actual assignment is `Vehicle* = Vehicle*` . – WhozCraig Aug 26 '21 at 18:19
  • `Vehicle* newVehicleObj` is not a `Vehicle` instance. It is a pointer to one. Note that in C++ you don't want to use `new` to allocate a class instance very often. `Vehicle newVehicleObj(9876,2021,200000,name,address);` is probably more along the lines of what you are looking for – user4581301 Aug 26 '21 at 18:20
  • 2
    Recommended reading: [Why should C++ programmers minimize use of 'new'?](https://stackoverflow.com/questions/6500313) – user4581301 Aug 26 '21 at 18:22
  • 1
    If you really want that pointer to pointer to be non-copyable, it needs to be enshrouded in a non-copyable entity. `std::unique_ptr` would probably do what you want (and as a bonus, also cleanup the blatant memory leak). – WhozCraig Aug 26 '21 at 18:25
  • Shouldn't the copy assignment have `Vehicle&` as the return type? – Lala5th Aug 26 '21 at 18:31
  • Quick hack demo of what is and isn't an `vehicle` instance and what can and can't be copied: https://godbolt.org/z/cGsf5s1h6 – user4581301 Aug 26 '21 at 18:37
  • 1
    @Lala5th • for an implemented one, yes, because that follows the conventional idiom. For the `= delete` case, doesn't matter (other than for consistency), since the return type isn't part of the signature, and might as well be `void`. – Eljay Aug 26 '21 at 19:11

2 Answers2

0

But still, below code is not showing error for copy assignment.

There is no error for assignment because you don't assign the Vehicle.

You do assign Vehicle*. There's no way to prevent assignment of pointers.

eerorika
  • 232,697
  • 12
  • 197
  • 326
0

Copy-ctor and copy-assignment overloads are generally instance to instance operations. Therefore, doing this:

Vehicle(const Vehicle&) = delete;
Vehicle& operator =(const Vehicle&) = delete;

disables these operations:

Vehicle obj1(args...);
Vehicle obj2 = obj1; // Error: copy-ctor deleted
Vehicle obj3(args...);
obj3 = obj1; // Error: copy-assignment deleted

But that isn't what you're trying to do. You are dynamically manual-allocating all your objects, then expecting to prevent pointer to pointer assignment with your overloads, specifically these:

Vehicle(const Vehicle*) = delete;
Vehicle* operator =(const Vehicle*) = delete;

Neither of these have anything to do with classic copy-ctor or copy-assignment. They're both user-defined overload operations; one a user-defined constructor for Vehicle that takes a Vehicle* as an argument, the other a user-defined assignment operator that likewise takes a Vehicle* as an argument. In short, "copying" has nothing to do with these. These only prevent the following (which (a) you're not doing, (b) nor should you be, and (c) is compile-time illegal anyway if you do not provide the aforementioned overloads, so there is no point to doing this in the first place):

Vehicle obj1(args...);
Vehicle *pobj2 = new Vehicle(args...);
obj1 = pobj2; // Error: operator declared deleted
Vehicle obj3(pobj2); // Error: user-defined ctor declared deleted

None of your actual posted code is actually using those operators.

What You Seem To Want

You seem to want to prevent both instance to instance copying and pointer to pointer copying. I've already showed you how to do the former. The latter requires you to enshroud your pointers in a non-copyable entity. Ideally said-entity would provide lifetime management as well, and as it turns out the standard library already provides such a class: std::unique_ptr<T>.

Keeping the instance copy protection is fine. But if your main code looked like this:

int main() 
{
   cout<<"\n +++ Checking for Vehicle Registration +++ \n";
   std::string name="shrikant";
   std::string address="Indi";

    // create a unique instance dynamically, owned by a unique_ptr
   auto newVehicleObj = std::make_unique<Vehicle>(9876,2021,200000,name,address);
   newVehicleObj->printDetails();

    // create another unique instance
   auto newVehicleObj1 = std::make_unique<Vehicle>(987,2022,300000,name,address);
   newVehicleObj1->printDetails();

   // try to assign one unique pointer from another
   newVehicleObj=newVehicleObj1; // Error

   // note that *moving* a unique pointer from one to another
   //  is still possible, and will destroy the target if it had
   //  a prior occupant. But you have to explicitly specify the
   //  move using std::move
   newVehicleObj = std::move(newVehicleObj1) // OK. 

   // Bonus: memory cleanup via RAII FTW.
   return 0;
}
WhozCraig
  • 65,258
  • 11
  • 75
  • 141