Прошу ревью кода (С++, stl, ~140 строк)

Написал реализацию графа в виде шаблона с использованием стандартных контейнеров. Ссылка.

Попинайте, пожалуйста, за возможные ошибки. Спасибо!

Пример использования

#include <iostream>
#include "graph.h"

using namespace std;

const int VERTICE_COUNT = 3;

int main()
{
    Graph<int> G(VERTICE_COUNT);
    for(int i = 0; i < VERTICE_COUNT; i++)
    {
        G.addVertice( ::make_shared<int>(i) );
    }
    for(int i = 1; i < VERTICE_COUNT; i++)
    {
        G.connect(0, i, i);
    }
    ::cout << G;
    return 0;
}
  • Вопрос задан
  • 4127 просмотров
Пригласить эксперта
Ответы на вопрос 5
WhiteD
@WhiteD
Специалист широкого профиля
Давно с C++ дел не имел, но вроде у вас перегруженный оператор вывода в поток с ошибкой. Он всегда выводит в cout, даже если левый опреанд будет файлом или еще каким потоком.
Ответ написан
Комментировать
Mezomish
@Mezomish
1. Строка 43: вы возвращаете «сырой» указатель, а не shared pointer — так и задумано?

2. Общее замечание: в единственном числе «вершина» будет «vertex».

3. Не реализован метод удаления вершины.
Ответ написан
Комментировать
xanep
@xanep
Кроме озвученного выше, еще добавлю
— Из конструктора можете смело выкидывать capacity.
— Переменные классы лучше именовать так, чтоб отличались от локальных — mVertices, vertices_, _vertices… whatever.
— Vertice* vertice(int index) const — это плохой метод. Константный метод, возвращающий неконстантный указатель — это явно что-то не то. Думаю, вам нужно возвращать значение, либо константную ссылку.
Ответ написан
AxisPod
@AxisPod
— Проблемы с именованием, используете C++, stl, так и именуйте в стиле stl, давайте методам осмысленные имена.
— Не забывайте о typedef для value_type и подобных
— Зачем тип вершины заворачивать в shared_ptr? Это приведет к оверхеду, это приведет к снижению производительности
— vertices.at(x); используется совсем не по назначению, если надо кинуть out_of_range, так и проверяйте с размером и кидайте руками.
— Как написано выше operator<< выводить должен в stream, а не std::cout
— Vertice* vertice(int index) const необходимо возвращать ссылку, либо кидать исключение out_of_range, а не указатель.
Ответ написан
@Razario777
В детали не вдавался, то что бросилось в глаза сразу:

if(x > y)
            return std::make_pair(y, x);
        else
            return std::make_pair(x, y);


заменить на
return x > y ? std::make_pair(y, x) : std::make_pair(x, y);


или на
return std::make_pair(x > y ? y : x, x > y ? x : y);


Так же кое где пишите auto, где то явно указываете например int? Должна быть единая стилистика.
Ответ написан
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы