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

C++ Address Book (2nd follow up)

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

Problem

This will be my third post of this program, I have received a lot of great feedback and have learned a lot from the community. I have edited this program a lot, and am here for another review to see if I got it correctly. I had a hard time implementing one minor fix with output stream delimiter which was suggested in an earlier answer, but I believe I fixed the inheritance issues. Any other fixes would be very appreciated:

Person.h

#ifndef PERSON
#define PERSON
#pragma once

#include 
#include 
#include 
#include 

class Person
{
    friend std::istream& operator>>(std::istream&,Person&);
    friend std::ostream& operator<<(std::ostream&, const Person&);
public:
    //comparison operator
    bool operator<(const Person&) const;
private:
    std::string name;
    std::string address;
};

#endif // !PERSON


Person.cpp

#include "Person.h"

bool Person::operator>(std::istream &in, Person &p){
    in >> p.name;
    getline(in, p.address);
    return in;
}

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


Address_book.h

#ifndef ADDRESS_BOOK
#define ADDRESS_BOOK
#pragma once

#include "Person.h"

class Address_book
{
    friend std::istream& operator>>(std::istream&, Address_book&);
    friend std::ostream& operator add_book;
};

#endif


Address_book.cpp

#include "Address_book.h"

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

//sort address book
void Address_book::sort() {
    std::sort(this->add_book.begin(), this->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) {
        std::cout << abook.add_book[i] << "\n";
    }
    return os;
}

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


main.cpp

Solution

This code is greatly improved! Well done. Still, there are some things that may help you improve your code.

Don't overspecify member functions

Within the Address_book.h file, the member functions are declared like this

void Address_book::resize(size_t entries);
void Address_book::sort();


But they should not include the class prefix in this context. Instead, write them like this:

void resize(size_t entries);
void sort();


Be aware of signed vs. unsigned

Within the Address_book.cpp file, both functions include a line like this:

for (int i = 0; i < abook.add_book.size(); ++i) {


However, size() often returns an unsigned number, so it would be prudent to make i unsigned as well. I'd recommend std::size_t as in

for (std::size_t i = 0; i < abook.add_book.size(); ++i) {


Use const where practical

Usually, an extractor (operator <<) does not alter the underlying object. This is true of your code as well, which is good. But to make it explicit, declare the parameter as const. That is, the friend function should be this instead:

std::ostream& operator<<(std::ostream& os, const Address_book& abook);


Avoid the use of this in member functions

The code current contains this function:

void Address_book::sort() {
    std::sort(this->add_book.begin(), this->add_book.end());
}


However, it's usually not a good idea to explictly write this unless you have to. I'd recommend shortening to this:

void Address_book::sort() {
    std::sort(add_book.begin(), add_book.end());
}


Think of the user

It's not so convenient to count the addresses manually first and then enter them. Why not have the computer count them? One simple way to do that would be to have a blank line or end of file signal the end of the input, avoiding the need for the user to manually count addresses first.

Code Snippets

void Address_book::resize(size_t entries);
void Address_book::sort();
void resize(size_t entries);
void sort();
for (int i = 0; i < abook.add_book.size(); ++i) {
for (std::size_t i = 0; i < abook.add_book.size(); ++i) {
std::ostream& operator<<(std::ostream& os, const Address_book& abook);

Context

StackExchange Code Review Q#124398, answer score: 2

Revisions (0)

No revisions yet.