2

I have a class hierarchy as shown in the example below, where a State contains a list of ZipCodes and a list of Citys, each of which contain pointers to the ZipCodes.

The goal is to be able to update the ZipCodes without needing to update Citys (or to create new instances of City).

The C++ code below meets this requirement, but it uses pointers, which I prefer to avoid because of this and that. How can I re-design this [naive] implementation so that it doesn't rely on pointers? Thanks for any help!

EDIT: Updated code below to use boost::shared_ptr instead of raw pointers. Note that State, City, and ZipCode are just example names, and they turned out to be poor choice names (I could've picked "A", "B", and "C") because the actual code allows the equivalent of City to share ZipCodes.

#include <iostream>
#include <vector>
#include <boost/shared_ptr.hpp>

using namespace std;

/**
 * Zone Improvement Plan (ZIP) code
 */
class ZipCode {
public:
    ZipCode() : code_(0), plus4_(0) {}
    ZipCode(int code, int plus4 = 0) : code_(code), plus4_(plus4) {}
    virtual ~ZipCode() {};

    int code() const { return code_; }
    int plus4() const { return plus4_; }
    void set_code(int code) { code_ = code; }
    void set_plus4(int plus4) { plus4_ = plus4; }

private:
    int code_;
    int plus4_;
};

typedef boost::shared_ptr<ZipCode> ZipPtr;

/**
 * City points to one or more zip codes
 */
class City {
public:
    const vector<ZipPtr>& zip() const { return zip_; }
    void add_zip_ptr(const ZipPtr x) { if (x != NULL) zip_.push_back(x); }

private:
    // TODO: this vector should be a hash set
    vector<ZipPtr> zip_;
};

/**
 * State contains cities, each of which has pointers to
 * zip codes within the state.
 */
class State {
public:
    const vector<City>& city() const { return city_; }
    const vector<ZipPtr>& zip() const { return zip_; }

    const ZipPtr zip_of(int code) const {
        for (size_t i = 0; i < zip_.size(); i++) {
            if (zip_[i]->code() == code) {
                return zip_[i];
            }
        }
        return ZipPtr();
    }

    void add_city(const City& x) { city_.push_back(x); }
    void add_zip(int code) { zip_.push_back(ZipPtr(new ZipCode(code))); }

private:
    // TODO: these vectors should be hash sets
    vector<City> city_;
    vector<ZipPtr> zip_;
};

int main() {
    State texas;
    City dallas, houston;

    // create state ZIPs
    texas.add_zip(75380);
    texas.add_zip(75381);
    texas.add_zip(77219);
    texas.add_zip(77220);

    // point city ZIPs to the ones we just created
    dallas.add_zip_ptr(texas.zip_of(75380));
    dallas.add_zip_ptr(texas.zip_of(75381));
    houston.add_zip_ptr(texas.zip_of(77219));
    houston.add_zip_ptr(texas.zip_of(77220));

    // print all ZIPs
    cout << "ZIPs in Texas: " << endl;
    const vector<ZipPtr>& zips = texas.zip();
    for (size_t i = 0; i < zips.size(); i++) {
        cout << "    " << zips[i]->code() << endl;
    }
    cout << "ZIPs in Dallas, Texas: " << endl;
    const vector<ZipPtr> zip_ptrs1 = dallas.zip();
    for (size_t i = 0; i < zip_ptrs1.size(); i++) {
        cout << "    " << zip_ptrs1[i]->code() << endl;
    }
    cout << "ZIPs in Houston, Texas: " << endl;
    const vector<ZipPtr> zip_ptrs2 = houston.zip();
    for (size_t i = 0; i < zip_ptrs2.size(); i++) {
        cout << "    " << zip_ptrs2[i]->code() << endl;
    }

    // change a state ZIP...
    cout << "Changing Houston's ZIP 77220..." << endl;
    ZipPtr z = texas.zip_of(77220);
    if (z != NULL) z->set_code(88888);

    // ...and show the ZIPs of the affected city
    cout << "ZIPs in Houston, Texas: " << endl;
    const vector<ZipPtr> zip_ptrs3 = houston.zip();
    for (size_t i = 0; i < zip_ptrs3.size(); i++) {
        cout << "    " << zip_ptrs3[i]->code() << endl;
    }

    return 0;
}
Community
  • 1
  • 1
  • 1
    As written the code is not correct: when you add a city or zip code to a state, the vector may need to reallocate itself, at which point all pointers to existing cities and zip codes would be invalidated (because the city and zip code objects will have moved elsewhere in memory). – James McNellis Feb 05 '11 at 03:57
  • @James, thanks for pointing that bug out. I'll have to edit –  Feb 05 '11 at 07:15
  • 1
    Since your topic is about pointer-avoiding programming paradigms, this might be of interest to you: [Nobody Understands C++: Part 6: Are You Still Using Pointers?](http://blog.emptycrate.com/node/354) (I like the catchy name.) I don't think it's entirely comprehensive, but it's a start :) – Domi Nov 25 '13 at 06:36

2 Answers2

1

I see the situation as two 1:n relationships

  1. State : City == 1 : n

  2. City : Zipcode == 1 : n

Based on that, I think that the State containing

vector<ZipCode> zip_;

is not sound.

I might do

class State {
    vector< City > cities_in_state_;
};

class City {
    vector< Zipcode > zips_in_city_;
};

This does not require pointers.

Arun
  • 19,750
  • 10
  • 51
  • 60
  • I agree with your comment on the 1:n in my example (and @zneak actually made the same comment). I was trying to pick a real-life example that was easier to understand than actual company-proprietary code (in which case these class relationships make sense), but I failed apparently :-\ –  Feb 05 '11 at 07:32
0

Unless you want to duplicate your ZipCode objects, you fall into this category of usage (described in your first link):

The Bar instance is actually managed by some other part of your program, whereas the Foo class just needs to be able to access it.

It seems like a legit use.

However, you might want to consider the copy option (to permanently avoid problems if the vector has to reallocate its data) or make State aggregate ZipCodes from its Cities instead of distributing them ZipCodes.

The copy simply implies that you stop using pointers in City. Aggregating the ZipCodes means that instead of giving State a list of ZipCodes, you would give City the list of ZipCode instances, and when calling zip_of, you would iterate through the cities and iterate through their ZipCode collection.

Community
  • 1
  • 1
zneak
  • 134,922
  • 42
  • 253
  • 328
  • Ok, thanks. Actually, the `State`, `City`, and `ZipCode` is a bad example (my bad). In my actual code, the equivalent of `City` can share `ZipCode`s. So, if I changed `City` to aggregate `ZipCode`s, there can be duplicate `ZipCode`s in `State`. –  Feb 05 '11 at 04:23
  • 1
    @helloworld The right way of sharing stuff with C++ are pointers or references. You could look into `std::shared_ptr` or `std::unique_ptr` from the C++0x STL to help. I haven't done enough C++ in my short life to have used it, but boost probably have equivalents of those too if C++0x isn't an option. – zneak Feb 05 '11 at 05:19
  • boost::shared_ptr looks promising...thanks for that. I might just use it –  Feb 05 '11 at 07:38