Динамические строки - дать оценку коду

@maksspace · Регистрация: 07.04.2014

Студворк — интернет-сервис помощи студентам

Всем привет. Только что написал пробную реализацию динамических строк на си. Товаарищи, дайте отзыв о этой библиотеке. Качество кода и так далее) Ссылка: https://github.com/maksspace/dynamic-string

HighPredator · 09.11.2015, 11:23

В общем глянул одним глазом. Сразу скажу, что встал с утра не с той ноги, так что не обессудьте.

Общие замечания:
1) названия хэдера и соответствующего сишника лучше заменить, чтобы у пользователя путаницы не возникало;
2) стиль кодирования хромает. Почитайте гайды, выберите что-то для себя и применяйте.
3) конст-корректность как следствие п.2
4) названия функций не точно отражают суть. Например функция str_delete на самом деле не удаляет строку, а освобождает память. Соответственно было бы логично назвать str_release к примеру. То же самое относится к функции конкатенации и другим;
5) нет механизма определения и обработки ошибочных ситуаций.

Частные:
1) функция

C

void str_delete(string_t * strptr)
{
    if(strptr != NULL)
    {
        free(strptr);
        strptr = NULL;
    }
}

содержит баг. Утекает память по указателю на данные.
2) напрасно не используете функции стандартной библиотеки в str_addstr, str_print и других.
3) функцию str_addchr было бы логично сделать с использованием str_addstr как частный случай. То же самое можно сказать про пару str_isequal/str_isequalchars.

Ну и вообще интуитивно не понятно как использовать. Вот я прочитал описание функции

C

/**
 * Make empty string
 * @param pointer to string_t
 * @return void
 */
void str_mkempty(string_t * strptr)

Ок, пробуем:

C

int main(void)
{
  string_t str;
 
  str_mkempty(&str);
  return 0;
}

Крэш... Такие дела.

@maksspace · 10.11.2015, 22:25 **[ТС]**

Меня уже ткнули носом на linux.org) Я переписал эту библиотеку и много что понял) если не сложно посмотри ее еще раз) ДУмаю теперь ситуация стала лучше)

https://github.com/maksspace/dynamic-string

Добавлено через 2 минуты
А почему стиль кодирования хромает)?

HighPredator · 11.11.2015, 12:00

Сообщение от maksspace

ДУмаю теперь ситуация стала лучше)

Имхо, стало только хуже... Стиль вообще превратился... Я даже не знаю, как это словами выразить. Вы что за образец взяли? Ассерты ни к селу, ни городу навтыкали повсюду... Жесть.

Продумайте архитектуру вцелом. А то сброд и шатание какое-то. Вот этот геморрой

C

1	int PrivateDynStr_CapacityResize(DynStr_t *str, size_t new_capacity)

вообще лечится ключевым словом статик. И не надо наворачивать приваты какие-то... Пораскиньте мозгами еще.

@maksspace · 11.11.2015, 15:48 **[ТС]**

Я про стиль честно говоря не понимаю, почему он стал еще хуже. Из-за длинных названий функций и не c-шного варианта именования? Для меня так strcasestr не чуть не понятнее чем DynStr_CaseStrCh, длиннее - очевидно да, но не понятнее. Какой стиль вы считаете хорошим? Если можно пример) Я думал как сделать назвать функции, что бы они давали ощущение какой-то высокоуровневости. А почему асерты не в тему? Там есть где нужно бы убрать, но они же не везде лишние? Например где указатели на строки ? Дайте пожалуйста пример хорошего стиля, я хочу понять что не так.

HighPredator · 11.11.2015, 16:17

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

C

int PrivateDynStr_CapacityResize(DynStr_t *str, size_t new_capacity)
/*...*/
void DynStr_AppendStrCh(DynStr_t *dst, const char *src)
/*...*/
void DynStr_AppendDYNSTR(DynStr_t *dst, const DynStr_t *src)

То же касательно параметров кстати.

Сообщение от maksspace

что бы они давали ощущение какой-то высокоуровневости

Вот этого не надо. Они должны:
а) быть понятными
б) работать
Все.

Что касается ассертов. Это средство отлова багов. Именно багов, а не ошибочных ситуаций. То есть, если у вас грамотно поставлена обработка ошибок, то пользователь может иметь возможность:
а) как-то своим способом их обработать и, по желанию, сохранить имеющиеся данные
б) продолжить выполнение в нужном ему ключе.
Ассерт же просто вырубит все к черту, что есть очень и очень плохо.

По поводу стиля, почитайте о стиле кодирования в гугле, id software. Тут на форуме по-моему где-то была тема с ссылками на всякие гайды. Поищите.

@maksspace · 11.11.2015, 16:32 **[ТС]**

А как вообще тогда правильно обрабатывать скажем нулевой указатель на строчку или что то совсем не валидное? Асерт понятно сразу закончит выполнять код, выдав сообщение с описанием ошибки. Получается входные данные лучше вообще не проверять внутри методов библиотеки, оставляя это на совести пользователя? или если и проверять то писать в stderr сообщение об ошибке ?

HighPredator · 11.11.2015, 16:50

Сообщение от maksspace

Получается входные данные лучше вообще не проверять внутри методов библиотеки, оставляя это на совести пользователя?

Почему, обязательно. Вопрос в том, что делать в каждом случае. Для этого, на этапе продумывания архитектуры каждой функции нужно определять все возможные ошибочные ситуации и решать, что делать в каждом конкретном случае. Например, вот у вас есть возможность там, где это уместно, возвращать коды ошибок. Так и сделайте это. Вам ничто не мешает.

@maksspace · 14.11.2015, 12:29 **[ТС]**

вот текущая версия: https://github.com/maksspace/dynamic-string. что скажете? )

@maksspace 0 / 0 / 0 Регистрация: 07.04.2014 Сообщений: 10
		1
	Динамические строки - дать оценку коду 08.11.2015, 22:21. Показов 908. Ответов 8 Метки нет (Все метки) Всем привет. Только что написал пробную реализацию динамических строк на си. Товаарищи, дайте отзыв о этой библиотеке. Качество кода и так далее) Ссылка: https://github.com/maksspace/dynamic-string 0

@maksspace 0 / 0 / 0 Регистрация: 07.04.2014 Сообщений: 10
	10.11.2015, 22:25 [ТС]	3
	Меня уже ткнули носом на linux.org) Я переписал эту библиотеку и много что понял) если не сложно посмотри ее еще раз) ДУмаю теперь ситуация стала лучше) https://github.com/maksspace/dynamic-string Добавлено через 2 минуты А почему стиль кодирования хромает)? 0

@maksspace 0 / 0 / 0 Регистрация: 07.04.2014 Сообщений: 10
	11.11.2015, 15:48 [ТС]	5
	Я про стиль честно говоря не понимаю, почему он стал еще хуже. Из-за длинных названий функций и не c-шного варианта именования? Для меня так strcasestr не чуть не понятнее чем DynStr_CaseStrCh, длиннее - очевидно да, но не понятнее. Какой стиль вы считаете хорошим? Если можно пример) Я думал как сделать назвать функции, что бы они давали ощущение какой-то высокоуровневости. А почему асерты не в тему? Там есть где нужно бы убрать, но они же не везде лишние? Например где указатели на строки ? Дайте пожалуйста пример хорошего стиля, я хочу понять что не так. 0

@maksspace 0 / 0 / 0 Регистрация: 07.04.2014 Сообщений: 10
	11.11.2015, 16:32 [ТС]	7
	А как вообще тогда правильно обрабатывать скажем нулевой указатель на строчку или что то совсем не валидное? Асерт понятно сразу закончит выполнять код, выдав сообщение с описанием ошибки. Получается входные данные лучше вообще не проверять внутри методов библиотеки, оставляя это на совести пользователя? или если и проверять то писать в stderr сообщение об ошибке ? 0

HighPredator 6044 / 2159 / 753 Регистрация: 10.12.2010 Сообщений: 6,005 Записей в блоге: 3
	11.11.2015, 16:50	8
	Сообщение от maksspace Получается входные данные лучше вообще не проверять внутри методов библиотеки, оставляя это на совести пользователя? Почему, обязательно. Вопрос в том, что делать в каждом случае. Для этого, на этапе продумывания архитектуры каждой функции нужно определять все возможные ошибочные ситуации и решать, что делать в каждом конкретном случае. Например, вот у вас есть возможность там, где это уместно, возвращать коды ошибок. Так и сделайте это. Вам ничто не мешает. 1

@maksspace 0 / 0 / 0 Регистрация: 07.04.2014 Сообщений: 10
	14.11.2015, 12:29 [ТС]	9
	вот текущая версия: https://github.com/maksspace/dynamic-string. что скажете? ) 0

Опции темы