if (order.getOrderCode() == null)
{
simpleOrder = true;
}
else
{
if (order.getOrderCode().length() == 0)
{
simpleOrder = true;
}
}
{
simpleOrder = true;
}
else
{
if (order.getOrderCode().length() == 0)
{
simpleOrder = true;
}
}
Refactor.
Note: There are plenty of libraries that offer very good single static operations to replace the if condition, but that's not important right now.
Is this a challenge, or an attempt to crowdsource your job? ;-)
ReplyDeleteFrom my ancient programming experience I would say that, at the very least, the nesting in the if/else construct is undesirable and a single compound conditional would be more efficient. (assuming that the second proposition in the condition is not tested if by implication the first proposition has already provided the outcome)
e.g. something like
if (a||b)
{
x = true;
}
Also, I do believe that an explicit assignment of "true" is unnecessary? As is the entire "if" conditional construct. One could assign the outcome of a conditional proposition directly? (especially since the comparative outcome is analogous to the assignment)
So something like
x = (a || b);
Ready For Comments.
Your first attempt is the proper solution.
DeleteI disagree on your second attempt, as it alters the behaviour of the original program. The original program will never set simpleOrder to false, yet "x =(a||b)" will, if a and b are false. It's a small point, but worth making, I think.
It illustrates the hazard of refactoring without tests.
By the definition of Refactoring, and limiting scope to the visible section of coe, I am forced to admit truth in your point. When Refactoring code, the program MUST retain the original program's behaviour.
DeleteThrough a swamp of logic and reasoning I would be able to argue the case for my choice (or at least disprove it's falseness based on your particular point), though in the end my argument would hinge on one particular point:
1) The assumption that the state of 'order'
1.a) be of particular value
1.b) the conditions in question attempt to disprove earlier value
1.c) further testing is useless once the original value be proven non-applicable.
In this case, if simpleOrder were not false to begin with, the assumption was it would be useless to check for order's existential state or length.