patterncppMinor
C++ Address Book (2nd follow up)
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
Person.cpp
Address_book.h
Address_book.cpp
main.cpp
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 // !PERSONPerson.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;
};
#endifAddress_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
But they should not include the class prefix in this context. Instead, write them like this:
Be aware of signed vs. unsigned
Within the
However,
Use
Usually, an extractor (
Avoid the use of
The code current contains this function:
However, it's usually not a good idea to explictly write
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.
Don't overspecify member functions
Within the
Address_book.h file, the member functions are declared like thisvoid 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 practicalUsually, 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 functionsThe 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.