[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