Замечания по программе Мини-словарь

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/vocabulary.cpp#L16-L24

• [ ] Словарь следует принимать по константной ссылке. find возвращает итератор, предоставляющий доступ к паре "ключ"-"значение". Перевод можно получить через it->second

• [ ] word следует передавать по константной ссылке.

• [ ] Можно результат в optional не оборачивать, а вернуть пустой массив

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/vocabulary.cpp#L26-L41

• [ ] Слово и словарь принимать по константной ссылке

• [ ] Вместо перебора с использованием итераторов проще использовать range-based for

• [ ] Имена it и it1 здесь выбраны неудачно. Легко перепутать, что есть что. Вместо возврата optional, можно для сигнализирования о не найденном результате возвращать пустой массив

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/vocabulary.cpp#L43-L56

• [ ] Vocabulary принимать по константной ссылке.

• [ ] Вместо возврата optional можно возвращать пустой массив.

• [ ] В c++ 17 появилась следующая форма if, с которой можно сделать код проще:

  if (auto it = dict.find("something"); it != dict.end())
  {
  // тут it указывает на найденный элемент
  }

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/translator.cpp#L108

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

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/translator.cpp#L84-L106

• [ ] Странным выглядит передача словаря по ссылке, допускающей его модификацию. Этого не ожидаешь от функции, сохраняющей словарь.

• [ ] Странным выглядит передача имени файла словаря по ссылке, допускающей модификацию.

• [ ] Если SaveVocabulary вернёт false, пользователь, впервые запускающий программу, не узнает, что что-то произошло не так, а программа уже завершит свою работу

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/translator.cpp#L63-L82

• [ ] От этой функции не ожидаешь, что она будет модифицировать словарь

• [ ] word лучше передать по константной ссылке

• [ ] Излишне копировать массив переводов из optional во временную переменную

• [ ] Идиоматичнее использовать выражение it != translation.end() вместо it < translation.end()

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/translator.cpp#L38-L61

• [ ] От этой функции не ожидаешь, что она будет изменять содержимое словаря при сохранении. Но передача по неконстантной ссылке об этом так и говорит.

• [ ] В c++ 17 есть так называемые structured bindings (https://en.cppreference.com/w/cpp/language/structured_binding):

map<string, int> dict;
...
for (const auto& [key, value] : dict)
{
    // внутри цикла key - ссылка на ключ, value - ссылка на значение
    cout << key << " " << value << "\n";
}

[alexey-malov]

https://github.com/AnyaGl/oop/blob/0acfca8f473924a65093fd773315b8e55ff8b1f8/labs/lab2/task3/translator/translator.cpp#L6-L36

• [ ] Если dictionary - это out-параметр, его следует очистить в самом начале.

• [ ] Выделите функцию Split, выполняющую разделение строки на массив строк по символу разделителю

• [ ] Код какой-то сложный. Попробуйте упростить