Замечания по Dictionary

[alexey-malov]

при работе с пользователем не воспринимать отличный от Да ответ как Нет, т.к. пользщователь потеряет результаты работы

[alexey-malov]

программа не спрашивает о сохранении изменний, если проигнорировать ввод последнего неизвестного слова

[alexey-malov]

k=0,75

[alexey-malov]

class DictionaryLogic
{
public:
    DictionaryLogic(std::string const& filename);
    ~DictionaryLogic();
    bool HasWord(std::string const& word);
    std::string GetValueAt(std::string const& word);
    void InsertPair(std::string const& first, std::string const& second);
    void SetSaveState(bool flag);
    bool GetSaveState();
    void SaveNewWordsInFile();
    std::map<std::string, std::string> DictionaryLogic::GetMapFromFile();
private:
    std::string m_filename;
    std::map<std::string, std::string> m_newWords;
    std::map<std::string, std::string> m_oldWords;
    bool m_needToSave;
};

методы, не изменяющие состояние словаря, должны быть объявлены константными

std::string GetValueAt(std::string const& word); - дать имя из предметной области void InsertPair(std::string const& first, std::string const& second) - аналогично void SetSaveState(bool flag) - есть ли необходимость в этом методе. Разве сам класс не может выставить это состояние? std::map<std::string, std::string> DictionaryLogic::GetMapFromFile(); - Нет необходимости в этом методе, т.к. используется только в тестах. Лучше тестировать класс методом черного ящика - зачем знать что там у класса внутри? Состояние объекта следует определять по его публичным методам.

[alexey-malov]

void DictionaryLogic::SaveNewWordsInFile()
{
    ofstream out(m_filename, ios_base::app);
    for (auto & pair : m_newWords)
    {
        out << pair.first << ":" << pair.second << endl;
    }
    out.close();
}

я бы ожидал от этого метода, что после сохранения новых слов в файл будет сброшен флажок необходимости сохранения, а также список новых слов опустошится. потому как если будет возможность сохранять состояние словаря в любой момент, а не только при выходе, то последующие сохранения будут дописывать и дописывать новые слова

[alexey-malov]

std::string DictionaryLogic::GetValueAt(std::string const& word)
{
    std::string result;
    try
    {
        result = m_oldWords.at(word);
    }
    catch (std::out_of_range const & e)
    {
        std::ignore = e;
    }
    return result;
}

неоднозначное решение. в случае получения перевода для отсутствующего слова возвращение пустой строки будет требовать от вызывающего кода обязательной проверки результата на пустоту.

Еще как вариант - возвращать boost::optional<string>. В этом случае вызывающий код сможет проверить "есть результат" или "нет результата", прежде чем использовать его. В общем-то, отсутствие слова в словаре - нормальная ситуация. В этом случае HasWord вообще не нужен:

auto translaton = dictionary.GetTranslation(word);
if (translation)
{
   cout << *translation;
}
else
{
  cout << "no translation. please enter translation or enter empty line to cancel";
}

[alexey-malov]

У классов должен быть префикс C

[alexey-malov]

Стоит получше определиться с терминами. Рекомендую следующее:

• Dictionary - словарь (у тебя назывался dictionary logic)
  • получить перевод фразы
  • добавить перевод фразы
  • сохранить новые слова в файл
  • узнать, есть ли несохраненные слова
• Dictionary UI - обеспечивает ввод-вывод данных от пользователя
  • запросить строку текста (фразу, которую надо перевести, либо перевод для неизвестной фразы). возвращает строку текста, либо признак завершения работы с программой
  • вывести перевод фразы
  • вывести сообщение о том, что фраза была сохранена
  • запросить подтверждение сохранения изменений
• DictionaryApp - выполняет интерпретацию данных, поступающих от пользователя, обмен данными со словарем, вывод информации пользователю

протестировать DictionaryApp будет легче, если подсунуть ему в роли исходных данных сконфигурированный словарь, а также пользовательский ввод, моделирующий ряд простых сценариев взаимодействия пользователя с программой.

гугли inverse of control, dependency injection

[alexey-malov]

k=0,85