RattleInGlasses / ps_oop

0 stars 0 forks source link

Замечания по реализации адресной книги #20

Open alexey-malov opened 8 years ago

alexey-malov commented 8 years ago
class CName
{
public:
    CName(std::string const & = "");
public:
    std::string GetValue() const;
    void SetValue(std::string const &);
public:
    bool operator ==(std::string const &) const;
    bool operator !=(std::string const &) const;
private:
    std::string m_data;
};

m_data - слишком абстрактное название SetValue - почему бы не сделать оператор =, принимающий std::string, а вместо GetValue - оператор const std::string&

Оператор == использовать для приблизительного сравнения неправильно. а то Иван Иванов == Иван, Лучше сделать метод bool Match(const std::string & name)

alexey-malov commented 8 years ago
class CPostAddress
{
public:
    CPostAddress(std::string const &city = "",
        std::string const &street = "", 
        std::string const &house = "", 
        unsigned apartment = 0);
public:
    void SetCity(std::string const &);
    void SetStreet(std::string const &);
    void SetHouse(std::string const &);
    void SetApartment(unsigned);
    std::string GetCity() const;
    std::string GetStreet() const;
    std::string GetHouse() const;
    unsigned GetApartment() const;
public:
    bool operator ==(CPostAddress const &) const;
    bool operator !=(CPostAddress const &) const;
private:
    std::string m_city;
    std::string m_street;
    std::string m_house;
    unsigned m_apartment;
};

Насчет сравнения замечание аналогичное, только аргументом Match должен быть CPostAddress

alexey-malov commented 8 years ago
class CSubscriber
{
private:
    typedef std::vector<std::string> EmailList;
    typedef std::vector<std::string> PhoneList;
public:
    CSubscriber(CName const &name,
        CPostAddress const &address = CPostAddress(),
        std::initializer_list<std::string> const &phones = {},
        std::initializer_list<std::string> const &emails = {});
public:
    CName const &GetName() const;
    void SetName(CName const &);

    size_t GetPhonesCount() const;
    std::string const &GetPhone(size_t) const;
    std::string       &GetPhone(size_t);
    void AddPhone(std::string const &);
    void DeletePhone(size_t);
    void SetPhone(size_t, std::string const &);
    bool HasPhones(std::vector<std::string> const &) const;

    size_t GetEmailsCount() const;
    std::string const &GetEmail(size_t) const;
    std::string       &GetEmail(size_t);
    void AddEmail(std::string const &);
    void DeleteEmail(size_t);
    void SetEmail(size_t, std::string const &);
    bool HasEmails(std::vector<std::string> const &) const;

    CPostAddress const &GetAddress() const;
    void SetAddress(CPostAddress const &);
private:
    CName m_name;
    PhoneList m_phones;
    EmailList m_emails;
    CPostAddress m_address;
};

Для доступа к номерам телефона лучше сделать необходимый набор методов. AddPhone DeletePhone UpdatePhone const vector<string> &GetPhones() Либо, если операций с телефонами становится много, то их можно выделить в отдельный класс

alexey-malov commented 8 years ago

alexey-malov commented 8 years ago
class CPostAddress
{
public:
    void SetCity(std::string const &);
    void SetStreet(std::string const &);
    void SetHouse(std::string const &);
    void SetApartment(unsigned);
...
};

Лучше давать параметрам имена.

alexey-malov commented 8 years ago
class CSubscriber
{
...
    bool HavePhones(std::vector<std::string> const &) const;
    bool HaveEmails(std::vector<std::string> const &) const;
}

Subscriber has phones.

alexey-malov commented 8 years ago

Для информации (tuple, tie, ignore):

#include <memory>
#include <string>
#include <iostream>
#include <chrono>
#include <iomanip>
#include <boost/format.hpp>
#include <vector>
#include <tuple>

using namespace std;

typedef tuple<int, string, double> MyTuple;

MyTuple ReadTuple()
{
    return make_tuple(14, "hello", 99.3);
}

int _tmain(int argc, _TCHAR* argv[])
{
    MyTuple t(10, "Hello", 3.15);

    cout << get<0>(t) << endl;

    pair<int, double> p(3, 8.66);
    cout << get<1>(p) << endl;

    int i;
    string s;
    double d;

    tie(i, std::ignore, std::ignore) = ReadTuple();

    cout << i << endl;

    return 0;
}
alexey-malov commented 8 years ago

Осталось сделать передачу имени файла