[P&AM Lab] (без темы)
Grigoriy A. Sitkarev
sitkarev на komitex.ru
Пн Апр 4 19:05:13 MSK 2011
Сейчас по порядку.
04.04.2011 07:09, Константин Никулов пишет:
> А чем плохи имена функций и макрос? Я лично вижу только то, что
> префикс можно укоротить.
Костя, понимаешь в чём дело, если бы ты знал в чём они плохи, то видимо
писал бы как-то иначе. Имена функций "плохи" только тем что не
соответствуют спецификации BLAS которой ты назвался (я об этом ниже).
Если вообще говорить то они выбраны хорошо.
С макросом плохо потому что ты его суёшь куда нужно и куда не нужно.
Например, вот твоя функция matrix_resize():
int
matrix_resize(matrix *mx, unsigned short rows, unsigned short cols)
{
double *res;
ptr_check(mx, MATRIX_UNSET)
if (mx->val == NULL) {
mx->val = malloc(rows * cols * sizeof(double));
ptr_check(mx->val, MATRIX_NOMEM)
}
res = realloc(mx->val, rows * cols * sizeof(double));
ptr_check(res, MATRIX_NOMEM)
mx->rows = rows;
mx->cols = cols;
return (matrix_clear(mx));
}
Здесь минимум одна серьёзная ошибка есть. Кроме того, код в этом случае
может быть проще (и должен):
int
matrix_resize(matrix *mx, unsigned short rows, unsigned short cols)
{
double *tmp;
size_t n;
ptr_check(mx, MATRIX_UNSET)
n = rows * cols;
tmp = realloc(mx->val, n*sizeof(double));
if (tmp == NULL)
return MATRIX_NOMEM;
mx->val = tmp;
mx->rows = rows;
mx->cols = cols;
matrix_clear(mx);
return mx;
}
Дальше, например в matrix_mult_matrix():
tmp1 = matrix_new();
ptr_check(tmp1, MATRIX_NOMEM)
tmp2 = matrix_new();
ptr_check(tmp2, MATRIX_NOMEM)
Это утечка памяти и это безусловно ошибка. Потому что должно было быть
примерно так:
tmp1 = matrix_new();
if (tmp1 == NULL)
return MATRIX_NOMEM;
tmp2 = matrix_new()
if (tmp2 == NULL) {
matrix_free(&tmp1);
return MATRIX_NOMEM;
}
Внимательно все функции нужно пересмотреть потому что есть ошибки. И это
не последние. Я просто все писать не могу. Поймите, если у вас код
скомпилировался это ещё ничего не значит, ничего абсолютно. Сейчас вы
просто что-то пишете и радуетесь если компилятор не ругался, но это
только первый шаг к тому чтобы код действительно стал работать и кто-то
мог на него положится в своих расчётах или программах.
Почему вы пишете сложно? Потому что вы зачастую не понимаете семантики
библиотечных функций, как это работает в действительности, у вас нет
полной ментальной модели. Поэтому я обхожусь вызовом одного realloc(3).
Что нужно сделать чтобы понимать семантику? Для этого нужно стать
Большим Читателем.
Сейчас вы многое просто копируете механически, не понимая смысла --
почему сделано так а не иначе? А нужно в первую очередь эти вопросы
задавать, тогда вы начнёте двигаться вперёд, потому что начнёте понимать
ЗАЧЕМ и ПОЧЕМУ, а это уже приближение к проектированию.
Макрос ptr_check() вероятно взят из macros.h который есть почти во всех
исходниках в репозитории, но он там полный и удобный и его назначение --
отлавливать ошибки программиста, а не ошибки которые возникают при
runtime. У нас они назывались return_if_fail и return_val_if_fail, т.к.
эти макросы делают return (что в общем случае категорически запрещается
для макросов) они могут использоваться только в самом начале функций и
только для проверки аргументов которые передавал туда пользователь.
Проверка аргументов вставляется только в public функции, т.е. те которые
идут в API библиотеки. В некоторых проектах макросы обрамлены ещё и
директивами препроцессора #ifndef NDEBUG которые выключают их если код
собирается без отладочных сообщений и т.д. Но runtime ошибки могут
возникать всегда и потому они этими макросами не обрабатываются.
Полнота этого макроса заключалась в том что он выводил место в котором
случилось безобразие и программист мог быстро найти и поправить свой код.
Макросы, которые занимают больше одной строки, настоятельно
рекомендуется заключать в конструкцию do { } while (0). Это позволит
избежать возможных трудно-обнаруживаемых ошибок при программировании. И
хотя в случае с макросом ptr_check() это может и не случится, в силу его
предназначения, такое обрамление всё равно нужно вставить.
> Ну, не BLAS так не BLAS. Спецификацию я еще почитать не успел, а мысль,
> что написать, имеется. Так почему бы и нет? По мере прочтения
> спецификации можно будет и BLAS писать.
Потому что здесь как в поговорке "назвался груздем -- полезай в короб".
Назвался BLAS-ом -- соответствуй спецификации.
Код возврата MATRIX_DONE режет глаз, почему он не назвался MATRIX_OK?
Если у тебя есть MATRIX_DONE, то подразумевается что матрица может быть
в некотором состоянии MATRIX_BUSY, раз код возврата читается как
завершение действия.
В программировании как и в архитектуре, нет ничего "просто так". И оно
не должно превращаться в карго-культ -- "делаем так потому что видели
что кто-то так делал, не понимая зачем".
--
Г.А.
Подробная информация о списке рассылки Lab