Opened 5 years ago

Closed 5 years ago

#158 closed ожидается проверка (задача сдана)

WW #3

Reported by: Igor Engel Owned by: Sokolov Viacheslav
Component: WW_intrusive_list Version: 2.0
Keywords: Cc:

Description

EOF надо как-то отдельно обрабатывать? Или предпологается что ввод завершается eixt'ом.

Change History (8)

comment:1 Changed 5 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

EOF можно считать UB.

get_length остался нереализованным.
В clist.c не хватает #include "clist.h"
Форматирование lst->head=to_add;
Сигнатуры не согласованы с точки зрения именования:
void add_node(intrusive_list* lst, intrusive_node* to_add);
void remove_node(intrusive_list* lst, intrusive_node* to_delete);

На самом деле у этих функций больше предусловий, и их можно проверить.

Чем мотивирован выбор long long в качестве хранилище длины?

Насчет macro command : кмк, лучше именовать ifcommand; зачастую у макросов есть определенное время жизни (когда они используются), в данном случае это int main(). Лучше определять макрос там, где он нужен, а после этого делать #undef, чтобы он не попадал туда, где он не нужен. То есть по сути использовать макросы так же, как используются переменные (с той разницей, что макрос - это переменная этапа препроцессирования).
Обычно макросы как-то выделяют в стиле написания, например, КАПСОМ (чтобы не путать их с функциями).

Чем мотивирован вызов calloc?

В main.c не хватает проверок предусловий функций.

getline? что это? :)

В текущей версии есть утечка памяти.

comment:2 Changed 5 years ago by Igor Engel

Type: ожидаются исправленияожидается проверка

Fixed
Оно там было...
Fixed
Fixed

Не совсем понял... Проверять что нода никуда не впихнута? Тогда надо требовать от вызывающего чистить её, что перекликается с пунктом про calloc... Проверять что при удалении впихнута в тот лист? Это надо за O(n) от головы смотреть...

Fixed

Поменял на malloc, но мне как-то спокойнее когда всё нули...

Вроде никаких особо нет... Лист никогда не исчезает... add, len, exit работают независимо, print выводит пустоту при пустом листе, rm(a) просто ничего не делают...

Очень красивый способ ввода) Но да, только на POSIX-системах. Надо совместимость под винду? Тогда придётся костыли на вводе городить...

Вроде все заткнул

comment:3 Changed 5 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления
Version: 1.02.0

Да, за O(len) проверять. В этом нет проблемы: это влияет на асимптотику debug сборки, но не release сборки. Мы пока не умеем проверять нормально некоторые контракты, например, что malloc вернет не NULL, в реальности же проверка контрактов будет устроена чуть-чуть иначе. Но проблемы в том, что контракт может быть небыстро проверить - нет.

В случае удаления можно проверить, что вершина есть в списке, в случае вставки - что нет.

А чем вызвано беспокойство?

Утечки удобно ловить в специальных режимах сборки и из-под valgrind. Утечка есть. Просьба воспользоваться одним из инструментов и найти ее.

Контрактов много. В этих двух строчках:

point* add_point(intrusive_list* lst, int x, int y) {
    point *p = malloc(sizeof(point));

явно предполагается, что:

  • указатель lst ненулевой
  • указатель p ненулевой

Про POSIX я обозначил в правилах: https://wiki.compscicenter.ru/index.php/C%2B%2B_1MIT_осень_1_2019

Все еще предлагаю именовать не COMMAND, а IF_COMMAND.

comment:4 Changed 5 years ago by Igor Engel

Ок. Сделал.


Меньше шансов где-то налажать

Вроде заткнул, в валгринде больше ничего не вижу.

Добавил.

Переписал.

А я всё ещё оставляю COMMAND, ибо декларативненько)

Last edited 5 years ago by Igor Engel (previous) (diff)

comment:5 Changed 5 years ago by Igor Engel

Type: ожидаются исправленияожидается проверка

comment:6 Changed 5 years ago by Sokolov Viacheslav

Type: ожидается проверкаожидаются исправления

Вопрос привычки и утилит: компилятор может выдавать предупреждения, если поле не инициализировано. Обратная сторона - может быть, где-то требуется какой-то метод инициализации, его вызов отсутствует, а эта проблема дольше остается незамеченной.
В общем хорошо бы добраться до C++, там этот вопрос решается однозначно.

Можно лучше декомпозировать: завести отдельную функцию "есть ли вершина в списке" и ставить assert(present) / assert(!present)

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

Границы между декларативным и императивным не очень четки. Разве if - else не декларативная конструкция? В этом месте в идеале должен быть switch, в каждом случае - описание. С моей точки зрения все наоборот - COMMAND - это приказ, повелительное наклонение, императив.

comment:7 Changed 5 years ago by Igor Engel

Type: ожидаются исправленияожидается проверка

Вынес

Добавил обработку

Ну, я это читаю как декларацию разных команд. Тоесть:

Команда exit: exited=1
Команда add: добавляем точку
Команда rm: удаляем точку

и так далее.

comment:8 Changed 5 years ago by Sokolov Viacheslav

Resolution: задача сдана
Status: assignedclosed
Note: See TracTickets for help on using tickets.