CODEWITHSUNDEEP
c++
user40120
user40120

Reputation: 648

Good Object Oriented Design When Deleting, C++

The code below deletes the symbol associated with the stock object just fine. Its just bad object oriented design.

The way that i am searching for each stock symbol is by using a != or == test for NULL.

bool hashmap::remove(char const * const symbol, stock &s,
int& symbolHash, int& hashIndex, int& usedIndex)
{
    if ( isAdded == 0 ) return false;
    if ( hashTable[isRemoved].symbol == symbol ) return true; 
    else 
    {
        symbolHash = this->hashStr( symbol ); 
        hashIndex = symbolHash % maxSize;
        usedIndex = hashIndex;
    }
    for ( int integer = 0; integer < maxSize; integer++ )
    {
        if ( hashTable[usedIndex].symbol != NULL &&
             strcmp( hashTable[usedIndex].symbol, symbol ) == 0 )
        {   
            isAdded--; 
            isRemoved = hashIndex; 
            s = &hashTable[usedIndex];
            delete hashTable[usedIndex].symbol; 
            hashTable[usedIndex].symbol = NULL; 
            return true;
            }
        ++usedIndex %= maxSize; // wrap around if needed
    }
    return false;
}

Im wondering now, if i delete in a such a way that:

hashTable[usedIndex].symbol = hashTable[NULL].symbol

Thereby, changing the way I logically test for an empty or found stock symbol. Is their a way to remove my stock symbol without having to redo the aspect of finding and searching?

Is this the correct way to remove in object oriented design?

Upvotes: 0

Views: 237

Answers (2)

Jared
Jared

Reputation: 6025

First of all, "object oriented design" is very ambiguous.

Is isAdded a member variable? It isn't clear that it is and creates another dependency on this function that isn't obvious when looking at the signature. Same thing goes for isRemoved.

Generally, a function that takes 5 arguments is getting close to showing that there is too much dependencies on this function (nevermind the invisible dependencies in isAdded, isRemoved, and hashTable).

I'm not sure what type hashTable is, but you should never have to call delete in 2009. You can use auto_ptr, shared_ptr, unique_ptr (in C++0x). These will take care of freeing your resource when it is not needed anymore. If you are using an STL container, then don't use auto_ptr though.

If you would like to use a hashtable in C++, you really should consider using a hash_map. That will be a much better implementation than 99.9999% of us can accomplish.

When using a hash_map, you can call void erase(iterator first, iterator last) to erase/delete an element.

Upvotes: 4

wallyk
wallyk

Reputation: 57774

How is an insert collision handled? Most standard solutions—such as a linear search for an open slot, or generating a new hash—make deleting an element problematic. If deletions are common, consider using a list structure instead.

Upvotes: 2

Related Questions