It made the code look a little messy, is easily resolved in the build
system and made the dependencies more complicated than needed. Each
GParted header was tracked via multiple different names (different
numbers of "../include/" prefixes). For example just looking at how
DialogFeatures.o depends on Utils.h:
$ cd src
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/../include/Utils.h \
../include/../include/../include/../include/../include/../include/Utils.h \
../include/../include/../include/Utils.h \
After removing "../include/" from the GParted header #includes, just
need to add "-I../include" to the compile command via the AM_CPPFLAGS in
src/Makefile.am. Now the dependencies on GParted header files are
tracked under a single name (with a single "../include/" prefix). Now
DialogFeatures.o only depends on a single name to Utils.h:
$ make DialogFeatures.o
$ egrep ' [^ ]*Utils.h' .deps/DialogFeatures.Po
../include/DialogFeatures.h ../include/Utils.h ../include/i18n.h \
When a base class destructor is virtual, derived class destructors are
also virtual [1] even if they don't have the virtual qualifier.
As the Operation destructor is virtual, derived Operation* classes
destructors are virtual too. Add virtual qualifier just to reflect what
the C++ language mandates the compiler implement.
[1] Derived class with non-virtual destructor
http://stackoverflow.com/questions/7403883/derived-class-with-non-virtual-destructor
Operation classes now internally use pointers to Partition objects and
take on management of their lifetimes. As before, with the
PartitionVector class, when storing pointers in a class the Big 3 of
destructor, copy constructor and copy assignment operator also have to
be considered.
First, all the Partition objects are allocated in the derived Operation*
class parameterised constructors and freed in the associated
destructors. However the Operation classes are never copy constructed
or copy assigned; they are only ever created and destroyed. Only
pointers to the derived Operations are copied into the vector of pending
operations. Therefore the copy construtor and copy assignment operator
aren't needed. To enforce this provide inaccessible private
declarations without any implementation so that the compiler will
enforce this [1][2].
This example code fragment:
1 OperationCheck o1( device, partition );
2 OperationCheck o2 = o1;
3 o2 = o1;
Does these OperationCheck calls:
1 Implemented parameterised construtor,
2 Disallowed copy constructor,
3 Disallowed copy assignment
Trying to compile the above code would fail with errors like these:
../include/OperationCheck.h: In member function 'void GParted::Win_GParted::activate_check()':
../include/OperationCheck.h:36:2: error: 'GParted::OperationCheck::OperationCheck(const GParted::OperationCheck&)' is private
OperationCheck( const OperationCheck & src ); // Not implemented copy constructor
^
test.cc:2:21: error: within this context
OperationCheck o2 = o1;
^
../include/OperationCheck.h:37:19: error: 'GParted::OperationCheck& GParted::OperationCheck::operator=(const GParted::OperationCheck&)' is private
OperationCheck & operator=( const OperationCheck & rhs ); // Not implemented copy assignment operator
^
test.cc:3:4: error: within this context
o2 = o1;
^
[1] Disable copy constructor
http://stackoverflow.com/questions/6077143/disable-copy-constructor
[2] Disable compiler-generated copy-assignment operator [duplicate]
http://stackoverflow.com/questions/7823845/disable-compiler-generated-copy-assignment-operator
Bug 759726 - Implement Partition object polymorphism
Win_GParted::Merge_Operations() method was modifying the internals of
Operation* objects; in particular the partition_new member variable.
This is breaking data hiding and encapsulation tenant of object oriented
programming.
Implement exactly the same operation merge semantics, but hide the
manipulation of the internals of the Operation* objects within the
Operation* classes themselves.
Bug 755214 - Refactor operation merging
Mike Fleetwood