LINUX.ORG.RU

Излишние проверки данных на валидность

 , ,


2

5

[offtop]С трудом подбираю нужные слова, чтобы не сорваться на мат.[/offtop]

Суть проблемы: излишние проверки в коде.
Пример:

SomeClass::someMethod();
ArrayType array;
ArrayElement element;
int i, j;
if( (array.width() > i) && (array.height() > j)
{
	element = array.element(i, j);
}

//...

Element ArrayType::element( int i, int j)
{
	Element element;
	if(( width() > i) && (height() > j))
	{
		element = m_data[i*height() + j];
	}
	return element;
}

Привер значительно упрощен.
В реальном коде данные путешествуют по методам и в каждом втором проверяется их валидность.

Итак, вопрос, как избавится от различных проверок данных на валидность, если они следуют через длинные цепочки методов.
Есть ли какая-то методология, паттерны, общепринятые практики?

P.S. По многочисленным просьбам радиослушателей:

class Element
{
public:
    Element():pointer(NULL){}
    void* pointer;
}

★★★★★

Последнее исправление: trex6 (всего исправлений: 3)

Ответ на: комментарий от vtVitus

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

В каком отношении состояние и данныe в этой терминологии? «Кто на ком стоял?» (с)

ratatosk
()
Ответ на: комментарий от andreyu

И что будет на выходе, если условие не выполнится?

Если это не примитивный тип, то будет объект, созданный с помощью конструктора по умолчанию. Сама по себе такая конструкция никакого криминала не содержит. см. QVariant, boost::function.

ratatosk
()
Ответ на: комментарий от KRoN73

Объект должен сам знать, что для него валидно, а что — нет. И все проверки должны быть в нём. Соответственно и в одном только месте будут.

Ну это вообще эпично. Представляю себе std::vector, разработанный в таком ключе.

ratatosk
()
Ответ на: комментарий от ratatosk

Ну это вообще эпично. Представляю себе std::vector, разработанный в таком ключе.

Почему бы и нет? Лишняя пара тактов на обращении к индексам погоды не сделает. Даже 40 лет назад это не было большим напрягом — возьми тот же Паскаль.

KRoN73 ★★★★★
()
Ответ на: комментарий от ratatosk

В каком отношении состояние и данныe в этой терминологии? «Кто на ком стоял?» (с)

Для тебя может быть новостью, но данные объекта определяют его состояние, а так же состояние зависимых объектов. :D
x меньше нуля, невалидное состояние объекта везде - проверка осуществляется поведением объекта, то есть должен быть метод validate().
В данном конкретном месте х должно быть больше 10. Проверка в данном конкретном месте.

vtVitus ★★★★★
()

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

no-such-file ★★★★★
()
Ответ на: комментарий от KRoN73

Всё равно не хочется платить за то, что не используешь. А использовать такой интерфейс (то есть, уповать на внутреннюю проверку) - это я себе слабо представляю. А если проверять везде, в каждом слое, при том, что (0 <= i < size) - инвариант, который можно проверить один раз установить и не нарушать, это еще и лишний код. Если в виде assert'ов внутри вектора - пожалуйста.

К слову о производительности - bounds-checking elimination - весьма насущная проблема оптимизаторов managed языков.

ratatosk
()
Ответ на: комментарий от vtVitus

данные объекта определяют его состояние

валидность данных определяется валидностью состояния

Wow.

должен быть метод validate().

Amen.

ratatosk
()

Кстати реальный случай отсутствия проверок как в библиотеке, так и в софте.

Библиотека требует один из параметров в котором возвращает объем необходимой памяти.
То есть типа такого. Это было очень популярно в 90-тые.

long memory;
execSome(NULL, &memory);
void * p = malloc(memory)
execSome(p, NULL);
...
free(p);

однако реализация библиотеки под саном очищала в переменной memory только первые два байта. Поэтому под саном усе падало через некоторое время из-за отсутствия памяти, а под линукс усе пахало как надо. и библиотека не поделие пионеров, а очень дорогой софт. Проверяйте параметры, если не хотите сидеть несколько недель, и понимать почему усе падает на пром. площадке.

vtVitus ★★★★★
()
Ответ на: комментарий от vtVitus

и библиотека не поделие пионеров, а очень дорогой софт.

разве качество софта определяется ценой?

Проверяйте параметры, если не хотите сидеть несколько недель, и понимать почему усе падает на пром. площадке.

ты опять всё вернул к тому с чего начали: в каком месте вставлять проверки. Но тут вообще странный случай ты рассказал. Имхо до такого уровня проверок лучше в коде не скатываться. Ты же не проверяешь что там вернул malloc. Вдруг он выделил перекрывающиеся области? Вдруг free() на самом деле не освободил память? Для таких вопросов лучше отдельные тесты чем городить огород в основном коде. Моё имхо.

true_admin ★★★★★
()
Ответ на: комментарий от true_admin

Для таких вопросов лучше отдельные тесты чем городить огород в основном коде.

Ты не сможешь в тесте отловить особенности реализация под разные платформы и конфигурации.

Имхо до такого уровня проверок лучше в коде не скатываться.

Как раз именно такие параметры и надо проверять.

то есть метод global_malloc(), который и должен проверять параметр и возврат малок и ругаться в лог, о подозрительном поведении.

Вдруг free() на самом деле не освободил память?

Ну это уже из области фантастики. :)

vtVitus ★★★★★
()
Ответ на: комментарий от true_admin

ты опять всё вернул к тому с чего начали: в каком месте вставлять проверки.

Проверки надо вставлять в тех местах, что б по ночам спать, а не сидеть в дебаге с трясущимися руками :)

vtVitus ★★★★★
()
Ответ на: комментарий от vtVitus

Проверки надо вставлять в тех местах, что б по ночам спать, а не сидеть в дебаге с трясущимися руками :)

У всех эти места разные :)

Ты не сможешь в тесте отловить особенности реализация под разные платформы и конфигурации.

так тесты надо на целевых платформах и конфигурациях запускать, не? Перед внедрением или после обновления, скажем, либ, ОС итп.

true_admin ★★★★★
()
Ответ на: комментарий от O02eg

В контексте данной дискуссии эти проверки лучше просто не вводить.

А вообще с помощью dependent types можно еще и не такие инварианты форсировать. Но...

Rule #1 of dependent types: never run your programs.

ratatosk
()
Ответ на: комментарий от ratatosk

Всё равно не хочется платить за то, что не используешь.

Повышение безопасности приложения и упрощение логики разработки — вполне ощутимый бонус за такую плату :) ИМХО, конечно.

инвариант, который можно проверить один раз установить и не нарушать

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

KRoN73 ★★★★★
()
Ответ на: комментарий от true_admin

У всех эти места разные :)

Да. Опыт диктует каждому свое. мой опыт диктует не доверять никому +) и проверять ну или хотя бы логировать.

так тесты надо на целевых платформах и конфигурациях запускать, не?

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

vtVitus ★★★★★
()
Ответ на: комментарий от vtVitus

Тесты не менее полезны именно как профилактика. Во-первых, имея тест, проще менять код, если накосячишь, может отловиться тестом. А во-вторых, а может даже и важнее, необходимость тестирования заставляет лучше продумывать интерфейсы, разбивать код на слабо связанные части, которые можно по отдельности тестировать.

ratatosk
()
Ответ на: комментарий от KRoN73

Я не понимаю логики, если честно. Если вдруг vector будет проверять индексы - чем от этого проще приложению? С неверными индексами правильно работать оно не начнёт.

ratatosk
()
Ответ на: комментарий от ratatosk

Если вдруг vector будет проверять индексы - чем от этого проще приложению?

Тем, что этим не придётся заниматься приложению.

С неверными индексами правильно работать оно не начнёт.

Нет. Но выбросит нормально перехватываемое исключение. С неверными индеками ничто нормально работать не начнёт, с этой точки зрения их можно вообще не проверять.

KRoN73 ★★★★★
()

По вопросу. Если у кода нет трудностей с производительностью, то трогать его нет причины.

Если нужно с нуля написать интерфейс, то я тут вижу два решения.

1. Интерфейс с проверками и без. Например в std::vector operator[]() не делает проверки индекса, at() делает и клиент сам выбирает, какое ему нужно поведение.

2. Параметр шаблона делать проверки или нет, при инстанциировании шаблона выбирать, какое требуется поведение. Это то, что у Александреску называется программирование на основе стратегий.

P.S. Если кто-то очень умный за меня решит нужны ли мне проверки при обращении к публичным методам класса (например, проверка границ индекса в std::vector), то такой негибкий интерфейс приведет к тому, что в каком-то случае он мне не подойдет и я напишу свой класс и выкину старый. Уверен, я не один такой.

x_hash
()

Вместо индексов i,j передавайте в методы спец класс индеков в котором производится валидация.

class ArrayIndex {
public:
ArrayIndex(int i, int j, const ArrayType& arr) : i_(i), j_(j), array_(arr) { validate(); }

int i() const { return i; }
int j() const { return j; }

bool validate() {
return valid_=(array_.width() > i) && (array_.height() > j);
}

bool valid() const { return valid_; }
bool seti(int i) { 
i_=i;
return valid_=valid_ &&  array_.width()>i_
}

// и т.п.

private:

int i_,j_;
ArrayType& array_;
bool valid_;
};

Итог:

1.Индекс валидируется один раз при создании, если элементы не меняются, ревалидация не проводится.

2.Если какие-то элементы меняются, проводится ревалидация только по этим элементам.

3.В коде массива никакая валидация не требуется - код чище.

4.Набор элементов индекса, принципы индексирования и валидации можно легко изменить, не затрагивая код класса массива.

no-such-file ★★★★★
()

Проверяться на валидность данные должны только там, где они реально используются. В твоем случае - в геттере контейнера.

Dragon59 ★★
()
Ответ на: комментарий от Dragon59

Проверяться на валидность данные должны только там, где они реально используются.

Это зачастую приводит к печальным последствиям когда программа падает не сразу, а с некоторой задержкой. И бэктрейс и эксепшены в таком случае для отладки бесполезны. Поэтому fail early, fail loudly.

PS лично я за то чтобы проверок и тестов было много, по крайней мере пока не будет достигнута стабильность приложения. Ну и за кнопочку в IDE «скрыть ассерты чтобы глаза не мусолили». Для особого упорина их можно вычищать для релизных исходников :)

true_admin ★★★★★
()
Ответ на: комментарий от true_admin

Если матрица не меняет своих размеров :)

Ну в stl при изменении размеров контейнеров итераторы могут стать не действительными и это никого не напрягает. А так, можно же прикрутить к этому делу оповещение объектов индексов об изменении размера массива с помощью механизма сигналов/слотов, например boost.signals или что-то типа этого. Можно навелосипедить и свою систему «сигналов» - хранить в объекте массива список всех его индексов и вызывать на них метод validate() после «опасных» операций.

В общем это уже технические детали. Кстати об stl. Можно в класс ArrayIndex добавить методы next()/prev()/begin()/end() для перебора элементов так, как будто массив одномерный. А тут уже и до stl итераторов и применения алгоритмов недалеко.

no-such-file ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.