Прошу ревью кода (С++, 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;
}
  • Вопрос задан
  • 4131 просмотр
Пригласить эксперта
Ответы на вопрос 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? Должна быть единая стилистика.
Ответ написан
Ваш ответ на вопрос

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

Похожие вопросы