Codereview

[agbragin]

No description provided.

[matklad]

Для таких задач класс это overkill

[matklad]

Коменты, дословно повторяющие код не нужны.

[matklad]

Тут не нужна вложенность.

Вообще, можно и нужно делать так

with open('foo.txt') as f:
    data = f.read()

print(data) # и вот тут data определенна всегда, несмотря на то, что она объявляется во вложенном блоке

[matklad]

Хорошо!

• Будь проще -- не для каждой задачи нужно писать отдельный класс. В Питоне очень мощные встроенные структуры данных(dict, list, set, tuple, collections.namedtuple) и с ними можно писать простой и понятный код. А если используешь классы, то лучше продумывай интерфейс.
• Не забывай про то, что пустая строка и пустой контейнер это False
• Юнит-тесты это хорошо =) Но для маленьких задач есть более легковесный способ самопроверки

def is_prime(n):
    some code

assert is_prime(997)
assert not is_prime(42)
assert not is_prime(1)

• Не то чтобы это сильно относится к данному случаю, но вообще про комментарии есть одно важное замечание, про которое редко говорят: если хочется прокомментировать какой-то кусок кода, то лучше переписать его так, чтобы комментарии не были нужны.
• Замечание "классы тормозят, поэтому давай-те ка напишем что-то непонятое" мне кажется странным -- вроде там нечему тормозить. Если будет время, попробую с этим разобраться, но не обещаю -- понапосылали тут на review по 29 файлов =)

[agbragin]

Алексей, большое спасибо за анализ и замечания! Код исправлю в соответствии с комментариями и перевыложу.

Bottleneck в graphs/components.py постараюсь найти самостоятельно)

Представление графа в виде массивов head, edge, next - это не совсем что-то непонятное=), эта реализация достаточно часто используется (описана, например, тут); на Java она ощутимо быстрее, чем вариант, когда для хранения списков смежности используются объекты.

[matklad]

Представление графа в виде массивов head, edge, next - это не совсем что-то непонятное=)
По крайней мере для меня она была новая, спасибо.

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

Это сомнительно. В статье head edge next сравнивалась с хранением списка смежности в виде двусвязного списка(узлы с указателями). Это не совсем честно, потому что списки тормозят сами по себе. Честнее было бы сравниваться не со списком, а с вектором(расширяющимся массивом). И в этом случае производительность была бы примерно одинаковая, при том что код был бы такой же, как и для связного списка. Так что, прежде чем говорить что что-то тормозит, надо озаботится выбором оптимального конкретного типа данных(ArrayList vs LinkedList).

Другой вопрос в том, что в Java из коробки нет коллекций для примитивных типов, и ArrayList будет тормозить(возможно, но вроде мне это никогда не мешало) просто из-за boxingа.

Третий вопрос заключается в выборе абстрактного типа данных для хранения "списков" =)
Дело в том, что список, на самом деле, не нужен, а нужно множество. И, если в качестве реализации множества выбрать HashSet, то у списка смежности получатся такие же константы по времени, как и у матрицы смежности.

На практике как правило хорошо использовать именно вектора, а не множества, потому что не всегда нужна операция "есть ли ребро между A и B", но у вектора константа меньше, чем у хэш сета

[matklad]

И не совсем корректно тут говорить об объектах, ничто не мешает head/edge/next решение обернуть в объект с красивым интерфейсом

[effect]

На счет ощутимо быстрее действительно сомнительно.
Но если гнаться за эффективностью и оптимизацией, то понятно что, скажем, при 100000 вершинах и 200000 ребер три массива (head, edge, next) известного размера, лучше чем 100000 списков - будь то LinkedList или ArrayList - в любом случае оверхед на хранение самих списков получится значительный.

[matklad]

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

по памяти overhead действительно будет в любом случае, а по времени думаю что не в любом.

В случае с head/edge/next расположение соседей вершины в памяти зависит от порядка, в котором добавлялись рёбра, и, вообще говоря, не будет cache-friendly.
А в случае ArrayList соседи вершины лежат в одном массиве, что хорошо для обходов.

[matklad]

Хм, а не может ли тут получится, что одна вершинка добавляется в deque несколько раз?

[matklad]

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

Сделал глупый бенчмарк на Java: https://gist.github.com/matklad/3212a661cc3d6c7adca4
Мерил список смежности на ArrayList(1), head,edge,next представление(2), и список смежности на массивах интов(3).

(2) и (3) примерно одинаковые по времени работы, (1) раза в два медленнее.