2014-08-18 1 views
1

Рассмотрим следующий код (который является упрощенной версией исходной):Должен ли я создать класс вместо повторяющихся операторов get/set?

#include <string> 

namespace my 
{ 
    class CType; 
} 

class my::CType 
{ 
private: 

    std::string simple_name; 

public: 

    explicit CType(std::string simple_name) 
    : simple_name{std::move(RequireIdentifier(simple_name))} // Duplicate Code 
    { 
    } 
    std::string get_simple_name() const 
    { 
     return simple_name; 
    } 
    void set_simple_name(std::string value) 
    { 
     simple_name = std::move(RequireIdentifier(value)); // Duplicate Code 
    } 
}; 

RequireIdentifier функция возвращает ссылку на value где value не пусто в противном случае он будет бросать std::invalid_argument исключение.

У меня слишком много классов, которые аналогичны определению CType класса, все из них имеют закрытую переменную типа std::string, а затем они проверяют его значение не пусто со следующим выражением:

std::move(RequireIdentifier(value)) 

Вот моя проблема, она повторяется во всех этих классах! предположим, что у меня есть 10 классов, подобных CType, тогда должно быть 20 = 2 * 10 выражений, подобных приведенному выше случаю! я не хочу повторять аналогичный код во всех 20 местах кода! он ошибается.

Я думаю, что я должен определить новый тип класса:

class my::Identifier 
{ 
private: 

    std::string name; 

public: 

    explicit Identifier(std::string name) 
    : name{std::move(RequireIdentifier(name))} // Duplicate Code 
    { 
    } 
    std::string get() const 
    { 
     return name; 
    } 
    void set(std::string value) 
    { 
     name = std::move(RequireIdentifier(value)); // Duplicate Code 
    } 
}; 

А затем изменить определение класса CType как следующий код:

class my::CType 
{ 
private: 

    Identifier simple_name; 

public: 

    explicit CType(Identifier simple_name) 
    : simple_name{std::move(simple_name)} 
    { 
    } 
    Identifier get_simple_name() const 
    { 
     return simple_name; 
    } 
    void set_simple_name(Identifier value) 
    { 
     simple_name = std::move(value); 
    } 
}; 

BTW, я не могу избежать повторяющийся раздел std::move, и это не проблема.

Но если я не перегружать = оператор Identifier класса, и если я не объявить неявное преобразование в std::string для Identifier класса, то Identifier класс не будет работать как string класса. Тогда это не плохой выбор дизайна? Каким образом можно избежать повторного кода в таком случае (AFAIK, мы не должны подклассы из любого контейнера STL)?

EDIT: функция RequireIdentifier определяется как следующий код:

inline std::string& my::RequestIdentifier(std::string& value) 
{ 
    for(char c : value) 
    { 
     if(!isalnum(c) && c != '_') 
     { 
      throw std::invalid_argument{""}; 
     } 
    } 

    return value; 
} 

inline std::string& my::RequireIdentifier(std::string& value) // HERE IS IT 
{ 
    if(value.empty()) 
    { 
     throw std::invalid_argument{""}; 
    } 
    else 
    { 
     return my::RequestIdentifier(value); 
    } 
} 

Заключение

Как вы можете видеть, если я поставил «извлечение» из основной строки в моем CType классе (как указано в принятом ответе Tony D), и если я хочу работать с типом string, тогда я должен вызвать функции и set функций Identifier класс во всех моих 10 классах повторяется, так как я назвал функцию RequireIdentifier. Поэтому создание класса только для устранения избыточности одного вызова функции, технически не является хорошим подходом.

С другой стороны, и независимо от кода избыточности, я должен объявить Identifer класс, если я действительно думаю, что это новый тип, что string тип не может представлять это хорошо, и я могу объявить неявное определенный пользователем конструктор для того, чтобы сделать его совместимым с типом string, и, наконец, цель класса Identifier не должна быть аксессуаром get и set для типа string, его назначение должно быть нового типа.

+0

Какова функция причина 'RequireIdentifier'? Кажется немного странным, что он возвращает то, что вы пытаетесь переместить. Что произойдет, если вы дважды перейдете из одной и той же lvalue? – juanchopanza

+0

@juanchopanza: Он определен, чтобы избежать повторения проверки, если строка пуста. Я включил его определение. –

+0

Класс с чистыми функциями set/get над инженерами, просто структуры должно быть достаточно? или просто строка должна быть достаточно? – billz

ответ

1

Но если я не перегрузить оператор = класса идентификатора, и если я не объявить неявное преобразование в станд :: строка для класса идентификатора, то идентификатор класса не будет работать как Строковый класс. Тогда это не плохой выбор дизайна? Каков правильный способ избежать повторного кода в таком случае?

Так ... просто поставить "добычу" подстилающего string в вашем CType классе:

class my::CType 
{ 
    private: 
    Identifier simple_name; 

    public: 
    explicit CType(Identifier simple_name) 
     : simple_name{std::move(simple_name)} 
    { } 

    std::string get_simple_name() const 
    { 
     return simple_name.get(); 
    } 

    void set_simple_name(std::string value) 
    { 
     simple_name.set(std::move(value)); 
    } 
}; 

UPDATE: в соответствии с просьбой в комментариях, ниже - иллюстрация использования check_invariants() функции. Там все еще есть избыточность, но вы можете повторить произвольное количество проверок без дополнительной модификации каждой мутирующей функции.

class my::CType 
{ 
    private: 
    std::string simple_name; 

    void check_invariants() 
    { 
     if (!is_identifier(simple_name)) 
      throw std::invalid_argument("empty identifier); 
    } 

    public: 
    explicit CType(std::string simple_name) 
     : simple_name{std::move(simple_name)) 
    { 
     check_invariants(); 
    } 
    std::string get_simple_name() const 
    { 
     return simple_name; 
    } 
    void set_simple_name(std::string value) 
    { 
     simple_name = std::move(value); 
     check_invariants(); 
    } 
}; 

... с ...

bool is_identifier(const std::string& s) 
{ 
    return !s.empty() && 
     (isalpha(s[0]) || s[0] == '_') && 
     std::all_of(s.begin() + 1, s.end(), [](char c) { return isalnum(c) || c == '_'; }); 
} 
+0

Вы поощряете определять новый класс для повторяющихся операторов get/set? –

+1

@ccsadegh Эти вещи - вопрос баланса ... определение класса для обеспечения таких инвариантов часто бывает разумным, хотя устранение одной избыточности строк не является убедительным. Примечательной альтернативой является добавление функции 'check_invariants()', которую вы вызываете в конце конструктора (ов) и оператора присваивания, который может проверять любое количество таких условий. Кроме того, 'get' и' set' часто указывают на «анти-шаблон» - возможно, такие имена могут быть ключами на карте, хранящей значения или любую из многих других моделей, - это трудно понять, не понимая ваше приложение .... –

+0

Включите ли вы пример функции check_invariants()? –

Смежные вопросы