HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

"Set" your expectations low for this ADT

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisyourlowadtexpectationsforset

Problem

For my third assignment in CS2, I was given the following:


A set is a special bag that does not allow duplicates. Specify each
operation for a set of objects by stating its purpose, by describing
its parameters, and by writing preconditions, postconditions, and a
pseudocode version of its header. Then write a C++ interface for the
set. Include javadoc-style comments in your code.


The difference of two bags is a new bag containing the entries that
would be left in one bag after removing those entries that also
occur in the second set. Design and specify a method difference() for
the ADT bag that returns as a new bag the difference of the bag
receiving the call to the method and the bag that is the method's one
argument. Include sufficient comments to fully specify the method.

Not too much to say, looking for a more efficient way to implement this perhaps. Feel free to rip it apart!

SetInterface.h:

```
#ifndef SET_INTERFACE
#define SET_INTERFACE

#include

/** @class BagInterface BagInterface.h "BagInterface.h"
*
Definition of BagInterface class template. /
template
class SetInterface
{
public:

/** Virtual destructor. */
virtual ~SetInterface() {}

/** Gets the current number of entries in this set.
* @return The integer number of entries currently in the set.
*/
virtual int size() const = 0;

/** Gets the maximum number of items allowed in a set
* @return The integer number of maximum items allowed in a set
*/
virtual int getCapacity() const = 0;

/** Sets the maximum number of items allowed in a set
* @post The capacity memeber of the Set instance will be set to
* whatever number was passed in
* @param num The maximum number of items that can be stored in a set
*/
virtual void setCapacity(int) = 0;

/** Sees whether this set is empty.
*
* @return True if the set is empty, or false if not.
*/
virtual bool isEmpty() const = 0;

/** Adds a new entry to th

Solution

Overall design:

SetInterface is unnecessary. Defining an interface just for the sake of hiding private data & methods might be popular in Java and such languages that rely heavily on dynamic dispatch, but not so much in C++.

Your interface makes the code a lot more complicated, from a maintenance standpoint, and spread-out through at least three files. It will also add a virtual table to your class, which has a performance impact and increases the size of the compiled binary. My advice it to just throw away SetInterface and keep a simple and straightforward Set template class.

Other details:

-
Your main header comment is outdated:

/** @class BagInterface BagInterface.h "BagInterface.h"  
 *  
 * Definition of BagInterface class template. */


You are not calling you class/header-file BagInterface.

-
I'd suggest to keep parameter names in function/method prototypes, as this adds to the documentation of the code.

-
Template parameter name for types is by convention just T, though ItemType is also quite descriptive.

-
toVector() can be optimized to avoid vector reallocations by simply declaring the vector with itemCount size:

std::vector setContents(itemCount); // <-- Allocate storage just once
for (int i = 0; i < itemCount; ++i) 
{
    setContents.push_back(items[i]);
}


However, you can also construct the vector from a range, so that loop can actually be consolidated into a single line:

std::vector setContents(items, items + itemCount);


-
Take a look at the Rule of Three. This rule is basically about how your class is going to behave regarding copy via operator = and copy constructors. You might wish to provide custom ones or even completely disallow copy and/or assignment.

Code Snippets

/** @class BagInterface BagInterface.h "BagInterface.h"  
 *  
 * Definition of BagInterface class template. */
std::vector<ItemType> setContents(itemCount); // <-- Allocate storage just once
for (int i = 0; i < itemCount; ++i) 
{
    setContents.push_back(items[i]);
}
std::vector<ItemType> setContents(items, items + itemCount);

Context

StackExchange Code Review Q#82413, answer score: 5

Revisions (0)

No revisions yet.