Simple address book in C++ - follow-up

Question

After posting my original address book program I took the responses and edited my code. I have used every suggestion given, the code works great, and now I am looking for further review and suggestions from the Code Review community.

Parent class (Person):

//parent class
#ifndef PERSON
#define PERSON
#pragma once

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>

class Person
{
public:
    //input person variable
    std::istream& read(std::istream&, Person &p1);
    //output person variable
    std::ostream& print(std::ostream& os, const Person &p1);
    //comparison operator
    bool operator<(const Person&) const;
private:
    std::string name;
    std::string address;
};

#endif 

Person member functions:

#include "Person.h"

//input for address book person element
std::istream& Person::read(std::istream& in, Person &p1) {
    std::cin >> p1.name;
    std::cin.ignore();
    std::getline(std::cin, p1.address);
    return in;
}

//operator for address book person element
std::ostream& Person::print(std::ostream& os, const Person &p1) {
    os << "Name: " << p1.name << "\nAddress: " << p1.address << "\n\n";
    return os;
}

bool Person::operator<(const Person& other) const {
    return name < other.name;
}

Child class (Address Book):

//child class
#ifndef ADDRESS_BOOK
#define ADDRESS_BOOK
#pragma once

#include "Person.h"

class Address_book : public Person
{
    friend std::istream& operator>>(std::istream&, Address_book&);
    friend std::ostream& operator<<(std::ostream&, Address_book&);
public:
    //set vector size to match appropriate number of entries
    void num_of_ent(int, Address_book&);
    void Address_book::sort(Address_book&,int);

private:
    std::vector<Person> add_book;
};

#endif

Address book member functions:

#include "Address_book.h"

//sets vector size, creates person elements of vector
void Address_book::num_of_ent(int entries, Address_book& abook) {
    for (int i = 0; i < entries; ++i) {
            Person p;
        abook.add_book.push_back(p);
    }
}

//fills address book
std::istream& operator>>(std::istream& in, Address_book& abook) {
    for (int i = 0; i < abook.add_book.size(); ++i) {
        abook.add_book[i].read(in, abook.add_book[i]);
    }
    return in;
}

//sort address book
void Address_book::sort(Address_book& abook) {
    std::sort(abook.add_book.begin(), abook.add_book.end());
}
//prints contents of address book
std::ostream& operator<<(std::ostream& os, Address_book& abook) {
    for (int i = 0; i < abook.add_book.size(); ++i) {
        abook.add_book[i].print(os, abook.add_book[i]);
    }
    return os;
}

Main:

#include <iostream>
#include <vector>
#include <string>
#include "Address_book.h"
#include "Person.h"

void pause();

int main() {

    int entries = 0;

    std::cout << "Enter number of entries to go in addressbook: ";
    std::cin >> entries;

    Address_book abook1;
    abook1.num_of_ent(entries, abook1);

    //input into addressbook
    std::cout << "Enter the name, followed by the address:\n";
    std::cin >> abook1;
    //sort addressbook
    abook1.sort(abook1, entries);

    //output addressbook
    std::cout << "\n";
    std::cout << abook1;

    pause();
}

void pause() {
    std::string pause;
    std::cout << "Press any key followed by enter to continue...";
    std::cin >> pause;
}

Answer 1

Misuse of inheritance

The huge, glaring problem I see with this code can be shown in one line:

class Address_book : public Person

This looks to me like a completely broken architecture. As a rule, public inheritance should follow the Liskov Substitution Principle (LSP), and this isn't even close¹.

Before Barbara Liskov formulated the substitution principle, the usual phrasing was that public inheritance should reflect an "Is-a" relationship. The LSP attempts to...tighten that a little to deal with a few odd cases like circles vs. ellipses, where you can say a circle is a specific case of an ellipse, but (since you can't always substitute a circle where an ellipse is required) you shouldn't normally use public inheritance.

In this case, however, we don't need the more careful formulation. An address book isn't a person, and isn't really even anything like a person. We don't need to look much further to see that this is use (abuse?) of inheritance is problematic at best.

Consistent interface

I'd also change the interface a little bit. Right now, you have read and write member functions for Person, and operator<< and operator>> for an address book.

I'd prefer to change this so the public interface is more consistent. Since the (more or less) standard way to do formatted I/O is to use operator<< and operator>>, I'd use those for both classes.

Resizing

Right now, to resize your vector, you have code like:

//sets vector size, creates person elements of vector
void Address_book::num_of_ent(int entries, Address_book& abook) {
    for (int i = 0; i < entries; ++i) {
            Person p;
        abook.add_book.push_back(p);
    }
}

I'm not fond of the name (at first blush, the name sounds more like it should be retrieving the current number of entries). I'd also prefer to use vector's resize() member instead. Finally, a member function that's going to modify an object should normally modify the object of which it's a member, not have some other object passed to it to modify.

void Address_book::resize(int entries) {
    add_book.resize(entries);
}

Size type

That brings us to the next point: the type for the size of a container is preferably container_type::size_type or if that's not easily available, you can usually use size_t as a fair approximation. So that means the resize should really be changed to:

void Address_book::resize(size_type entries) {
    add_book.resize(entries);
}

...where size_type is something like:

typedef vector<Person>::size_type size_type;

...or:

using size_type = vector<Person>::size_type;

formatting of output

A fairly minor point is that by convention writing an object to the stream should only write the content of that object to the stream, not write a delimiter that separates that object from the next. For example, here:

//operator for address book person element
std::ostream& Person::print(std::ostream& os, const Person &p1) {
    os << "Name: " << p1.name << "\nAddress: " << p1.address << "\n\n";
    return os;
}

...we really want to get rid of the trailing new-lines, and leave that to the client code.

std::ostream& Person::print(std::ostream& os, const Person &p1) {
    os << "Name: " << p1.name << "\nAddress: " << p1.address;
    return os;
}

Then we might use it something like this:

std::copy(my_book.begin(), my_book.end(), 
          std::ostream_iterator<Person>(std::cout, delimiter));

...and to get the same format, we'd have delimiter = "\n\n";, but if we wanted the output single-spaced, it'd be "\n", and so on.

---

^{1. Nor do I see any indication that this might be an exception to the rule.}