I tried to picture the solution before scrolling down past the initial block with the nested ifs. It got me thinking about "switch()", as seen in C, C++, and elsewhere. That in turn got me thinking about using an enum to pick up on whatever it is he's trying to do here, and jump to the right place accordingly.<p>Generically, I had something like this in my head:<p><pre><code> switch (row_type) {
case submittable: ...; break;
case checkable: ...; break;
...
}
</code></pre>
Then I started realizing that using a switch means you have isolated things such that they can only be one of the n types. It won't ever be submittable AND checkable, for instance. I assume that's what is desired here: mutual exclusivity for all of these possibilities.<p>Looking back at the original code, I see that by using the nested if/else construct, it effectively gives priority to them in the order in which they are encountered. A row with a submit_button is going to fire off make_submit_cell() whether or not it has checkable set. It trumps the possible call to make_check_cell, in other words.<p>These two approaches solve two different problems. Without knowing more, it's hard to say whether the "priority" approach makes sense here.<p>If I had to guess, based on the refactoring which turned it into a series of distinct classes, that sounds like it should have been "pick 1 of n" (one-hot code) all along. If that's so, the if block wasn't just ugly, but it was actually covering up some real problems. It was just asking for trouble in terms of bugs down the road since it allowed nonsensical settings.<p>But that's just a guess. I could be wrong.