2010-09-15 6 views
1

Какой способ написания этого условия лучше?Лучше использовать || или включать? при проверке переменной на несколько значений?

1)

(self.expense_gl_dist_code.dist_type == GlDistCode::PERCENTAGE || self.expense_gl_dist_code.dist_type == GlDistCode::MIXED) 

2)

["GlDistCode::PERCENTAGE","GlDistCode::MIXED"].include?(self.expense_gl_dist_code.dist_type) 
+2

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

+0

@ Daniel Vandersluis Извините за это ... –

ответ

2

Я нахожу второй понятнее по двум причинам:

1) Во втором варианте элементов проверяемых все рядом друг с другом, разделенные запятыми. В первой версии всегда есть self.expense_gl_dist_code.dist_type ==, так что их легко сканировать сразу.

2) Во втором варианте это сразу видно, что все элементы проверяются на том же состоянии, в то время как в первом варианте он может сказать что-то вроде

dist_type == GlDistCode::PERCENTAGE || dist_type == GlDistCode::MIXED || dist_type != GlDistCode::WHATEVER 

, и вы можете не заметить сразу.

+0

Считываемость - это FTW –

1

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

+1

Действительно ли это? Учитывая, что строка с переменным доступом длинна, я бы подумал, что это сломается, если у вас больше условий - да, первый из них более читабельен на данный момент, но представьте себе, что нужно проверить 10 различных возможностей. Метод «Array.include?» Вначале немного менее ясен, но как только вы выясните, как смысл теста как бы наоборот, это не проблема. –

+1

Возможно, здесь будет более подходящим заявление о переключении? – ennuikiller

1

Если вы просто сравниваете два элемента, я бы сказал, что все в порядке.

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

ALLOWED_TYPES = [GldDistCode::PERCENTAGE, GlDistCode::MIXED] 

затем

if ALLOWED_TYPES.include?(dist_type) 

более разборчивыми ИМХО.

Кстати, вы используете строки ("GldDistCode::PERCENTAGE") вместо фактического значения, которое вы намеревались.

Смежные вопросы