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;
}

★★★★★

И как, работает без коредампов? Помогло?

nerdogeek ()

Когда метод не знает, кто его будет вызывать и при каких обстоятельствах, то это вполне нормальная практика.

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

В итоге сотни проверок на валидность одних и тех же данных.
Ладно, когда этот код просто работает, но мне вот сейчас приходится разбираться с куском кода, где проверка одних и тех же данных производится раз 10, при этом для проверки используются 3-4 разные функции в разных классах. Это жестоко.

trex6 ★★★★★ ()

А чем они, собственно, тебе мешают?

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

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

nerdogeek ()

ИМХО:

Проще разбираться, если есть разделение на уровни. И функции/методы имеют определенные контракты, данные - определенные инварианты. Проверка валидности осуществляется на каком-то уровне, а дальше - только в виде assert'ов, чтобы ловить логические ошибки, приводящие к нарушению инвариантов/контрактов.

То есть, если по контракту при неправильном индексе массив возвращает пустышку (а ля null object), то и незачем это дублировать.

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

Проще разбираться, если есть разделение на уровни. И функции/методы имеют определенные контракты, данные - определенные инварианты. Проверка валидности осуществляется на каком-то уровне, а дальше - только в виде assert'ов, чтобы ловить логические ошибки, приводящие к нарушению инвариантов/контрактов.

Расскажите, какие книги вы читали, чтобы начать разговаривать таким языком? Обязательно положу их на полку для будущего прочтения.

То есть, если по контракту при неправильном индексе массив возвращает пустышку (а ля null object), то и незачем это дублировать.

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

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

Насколько я понял, это чужой вполне работающий код, и единственная его проблема — он задевает твоё чувство прекрасного. Может не стоит его трогать?

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

Надежность не определяется количеством проверок. А излишние проверки, на мой взгляд, свидетельствуют о промахах в архитектуре.

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

Я его трогать и не собираюсь.
Работает - не трогай(tm) - это мы уже впитали давным дано.

trex6 ★★★★★ ()

я бы написал так:

bool ArrayType::contains(int i, int j) {
 if( (width() < i) && (height() < j)
  return true;
 return false;
}

// пожалуй это ответ на твой вопрос - проверка нужна будет 
// только там где лезешь к данным, а в остальных местах только
// ловишь исключение
Element ArrayType::element( int i, int j) {
 if(!contains(i,j))
  throw NoElementException();
 return m_data[i*height + j];
}

тогда можно писать:
try {
 SomeClass::someMethod();
 ArrayType array;
 ArrayElement element;
 int i, j;
 element = array.element(i, j);
} ....

invy ★★★★★ ()

Я делаю так: проверки данных на валидность только в public-методах, т.к. хз, кто их вызовет. Если мне нужно вызвать public-метод из private, то выделю из public'a private-метод без проверок и буду дергать его.

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

это ты знаешь, откуда пришли данные в функцию и что они уже были провалидированы. а функция не знает. и валидировать может по другому критерию, нежели вызывающая

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

Да ничего такого из книжек не припомню, Code Complete возможно, хотя и не могу сказать, что эта книга мне шибко симпатична, дочитать ее ло конца - имхо подвиг, достойный Геракла. Еще есть хорошая книга Elements of Programming, но язык куда-а-а страшней... :)

http://en.wikipedia.org/wiki/Design_by_contract

Также, например, приятно читать доку Boost, где на каждую ф-цию указаны preconditions, postconditions.

ratatosk ()

Барьерный принцип. Всё, что взаимодействует с внешним миром данные проверяет, всё внутреняя (хорошо отлаженная) кухня - нет. Тогда проверки делаются (барьер) только в одном месте.

ArrayType::element

пример эталоннейшего быдлокода.

nanoolinux ★★★★ ()

Есть ли какая-то методология, паттерны, общепринятые практики?

Есть. Использовать type system.

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

пример эталоннейшего быдлокода

Почему? Неудачное название метода?

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

пример эталоннейшего быдлокода.

там ещё и возвращается копия, а не ссылка ;) только тссс :)

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

Использовать type system.

Можно поподробнее?

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

два ненужных копирования данных

Так ведь это просто пример. К тому же никто не говорил, что выше нет такого

typedef int Element;
или
typedef MegaClass Element;

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

bool ArrayType::contains(int i, int j) {
if( (width() < i) && (height() < j)
return true;
return false;
}

return width() < i && height() < j;

же

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

Я вообще привык писать

bool result = false;
if(...)
{
    result = true;
}

return result;

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

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

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

А у меня однострочники зачастую вырастают в полноценные функции, поэтому сразу все пишу в одном стиле =)

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

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

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

Имхо: лаконичная декларативная формулировка vs. порожняя императивная бородень. :)

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

да, можно и так, в данном случае даже лучше. сойдёт за отмазку: копипастил? :)

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

Ну блин. почему width() - метод, а height - нет? Что там конструктор Element по умолчанию делает? Что возвращается? Что с этим потом делать сверху?

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

Ну и возвращать Element по значению - моветон. Ладно бы там shared_ptr - я бы понял. Хотя если он - структура с двумя интами по сути, то может и ничего страшного.

nanoolinux ★★★★ ()

Поддерживаю разделение на внешний и внутренний код, как в Code Complete. И вообще, let it crash:-)

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

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

помноженое на

Что там конструктор Element по умолчанию делает?

доставит массу лулзов при поиске багов :)

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

Поддерживаю разделение на внешний и внутренний код, как в Code Complete. И вообще, let it crash:-)

Зря буржуйским балуеетсь, «позволь ему упасть» звучит куда интереснее и многозначительнее.

По сабжу: барьерный принцип, классное название и классный принцип.

P.S. а вообще я не люблю портить код там, где можно решить вопрос регулярными ревью кода, проверками компилятора и более аккуратным отслеживанием изменений в критичных местах.

quiet_readonly ★★★★ ()
Последнее исправление: quiet_readonly (всего исправлений: 1)
Ответ на: комментарий от nanoolinux

Ну блин. почему width() - метод, а height - нет?

Это не какой-то рабочий код, а пример, который я прямо тут писал. Имелось в виду height(), но поторопился, т.к. был в растроенных чувствах (см. offtop в первопосте).

Вышел за пределы - лови эксепшн.

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

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

Использовать type system.

Можно поподробнее?

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

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

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

trex6 ★★★★★ ()
	Element element;
	if(( width() < i) && (height() < j))
	{
		element = m_data[i*height() + j];
	}
	return element;

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

andreyu ★★★★★ ()
Последнее исправление: andreyu (всего исправлений: 1)
Ответ на: комментарий от nerdogeek

Ну, выглядит как попытка написать надежную софтину. Без багов. Совсем.

Вернуть неизвестно что, если условие не выполнилось, это без багов?

andreyu ★★★★★ ()

Итак, вопрос, как избавится от различных проверок данных на валидность

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

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

Добавил постскриптум в первый пост.

Здесь суть вообще не в Element и его поведении, а в проверке одних и тех же данных на валидность по одному и тому же условию нинесколько раз.

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

Компиляторы умеют такое оптимизировать. Впрочем, полагаться на это не стоит.

anonymous ()

Валидность данных должна проверяться в том месте, где известно о правилах валидности.

Если ожидаются только валидные данные. то должна быть проверка типпа assert

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

не учите детей плохому

ваш совет, в свою очередь, валиден только если вопрос производительности не рассматривается в принципе.

anonymous ()
Ответ на: не учите детей плохому от anonymous

Use exceptions - невалидные данные это исключительная ситуация, по крайней мере так должно быть.

batbko ()
Ответ на: не учите детей плохому от anonymous

Re: не учите детей плохому

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

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

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