Timeline for How do I edit a chain of if-else if statements to adhere to Uncle Bob's Clean Code principles?
Current License: CC BY-SA 3.0
29 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Jan 15, 2018 at 6:06 | comment | added | candied_orange | loc? you mean Lines Of Code? | |
| Jan 14, 2018 at 17:50 | history | edited | Robert Harvey | CC BY-SA 3.0 |
EDIT: monikers are not required. Every post on Stack Exchange has a detailed edit history that anyone can view.
|
| Jan 13, 2018 at 19:38 | comment | added | Ewan | the loc is static in regard to the number of conditions. where as the if block grows in proportion. | |
| Jan 13, 2018 at 17:50 | comment | added | candied_orange | Also, I don't see how any of this is a big O improvement. | |
| Jan 13, 2018 at 17:49 | comment | added | candied_orange | If the only objective is to make it shorter this is the wrong site. Take it to codegolf. We care more about readability here. | |
| Jan 13, 2018 at 15:08 | comment | added | 17 of 26 | The OP isn't happy with his code because he's misguided and is under the incorrect impression that shorter is always better. The best answer to his question is 'stop it, that code is fine as-is and trying to make it shorter makes it less readable'. | |
| Jan 13, 2018 at 0:15 | comment | added | Ewan | @williamperron My view on questions is that we have to take the example as an example. Im assuming that the OP isnt happy with his current code and wants suggestions. Just saying 'no you are fine' dosent help anyone. Assume that the number of else if statements is more than you are comfortable with. what do you do? | |
| Jan 12, 2018 at 21:11 | comment | added | William Perron | @Ewan this might be just me, but sacrificing readability simply to "improve" the big O notation of an algorithm simply isn't worth it. I put the wourd "improve" in air quotes hear because, again IMO, O(n) that can run a maximum of 4 times is perfectly acceptable. | |
| Jan 12, 2018 at 21:06 | comment | added | Ewan | @william yes, but it is O(1) rather than O(n) loc as it were | |
| Jan 12, 2018 at 21:03 | comment | added | William Perron | @Ewan I agree, what is or is not readable is somewhat subjective. However, each of your example is longer than the original code (and less readable IMO) and as such probably isn't best solution for OP | |
| Jan 12, 2018 at 16:07 | comment | added | Ewan | @CandiedOrange At some point someone at Microsoft decided that annotations were amazing and spent millions making us put them everywhere. So at least one person liked them at one point!! | |
| Jan 12, 2018 at 15:56 | comment | added | candied_orange |
Readability is NOT personal. It is a duty to the programming public. It is subjective. Which is exactly why it's important to come to places like this and listen to what the programming public has to say about it. Personally I find this example incomplete. Show me how conditions is constructed... ARG! Not annotation-attributes! Why god? Ow my eyes!
|
|
| Jan 12, 2018 at 15:51 | history | edited | Ewan | CC BY-SA 3.0 |
added 176 characters in body
|
| Jan 12, 2018 at 15:42 | history | edited | Ewan | CC BY-SA 3.0 |
added 176 characters in body
|
| Jan 12, 2018 at 15:35 | comment | added | Matthieu M. | Unless the list of conditions is dynamic, or can be encoded in a data-centric way, I would really rather favor hard-coded if-else. | |
| Jan 12, 2018 at 15:04 | comment | added | cmaster - reinstate monica |
@StephenC.Steel If you statically initialize the table with one line for each condition/action pair, how is that different from writing the if...else ladder? The ladder is well recognized code syntax, the table adds a level of indirection. If you are not able to somehow simplify the conditions/actions themselves, or to reduce their number by generating them programatically (exploiting redundancies between the different conditions/actions), I fail to see any gain in the table over the ladder. The table would just add complexity via indirection, no matter how long the table/ladder is.
|
|
| Jan 12, 2018 at 14:42 | comment | added | Ewan | @snb readability is a personal judgement call, what uncle bob thinks requires divine intervention. the OP wants shorter code | |
| Jan 12, 2018 at 14:31 | comment | added | Stephen C. Steel | Converting an if .. else if .. else .. chain to a table of predicates and actions makes sense, but only for much larger examples. The table adds some complexity and indirection, so you need enough entries to amortize this conceptional overhead. So, for 4 predicate/action pairs, keep the simple original code, But if you had 100, definitely go with the table. The crossover point is somewhere in between. @cmaster, the table can be statically initialized, so the incremental overhead for adding a predicate/action pair is one line that merely names them: hard to do better. | |
| Jan 12, 2018 at 14:17 | comment | added | Krupip | @Ewan Uncle Bob would never support making it short via making it less readable, and there are methods he provides for you to refactor, this is not one of them, you understood the question wrong. | |
| Jan 12, 2018 at 13:52 | comment | added | Ewan | maybe read the question again "I'm trying to follow Uncle Bob's clean code suggestions and specifically to keep methods short." | |
| Jan 12, 2018 at 13:51 | comment | added | 17 of 26 | @Ewan except that I think the whole point of his question is that he's looking for a way to make the code better. | |
| Jan 12, 2018 at 13:10 | comment | added | Ewan | @17of26 maybe, but the OP is asking about a specific way of writing their code. Lets not judge it | |
| Jan 12, 2018 at 13:02 | comment | added | 17 of 26 | This is less readable than the original. In order to figure out what condition is actually getting checked, you need to go digging in some other area of the code. It adds an unnecessary level of indirection that makes the code harder to understand. | |
| Jan 12, 2018 at 12:23 | comment | added | Ewan | @cmaster yes I think i did say exactly that "then the setup for the object will be as complicated as the original if statement£ | |
| Jan 12, 2018 at 12:06 | comment | added | cmaster - reinstate monica |
This only moves the if...else ladder into the construction of the Conditions list. The net gain is negative, as the construction of Conditions will take just as much code as the OP code, but the added indirection comes with a cost in readability. I'd definitely prefer a clean coded ladder.
|
|
| Jan 12, 2018 at 11:54 | comment | added | Ewan | edited to reflect your concern | |
| Jan 12, 2018 at 11:42 | history | edited | Ewan | CC BY-SA 3.0 |
added 729 characters in body
|
| Jan 12, 2018 at 9:31 | comment | added | Neil | It should be mentioned that this only works because the condition and the action to be performed are strongly related. If an additional action were required for one particular case, consider leaving this code intact and adding an if below for the special case in order to run this added action. | |
| Jan 12, 2018 at 8:39 | history | answered | Ewan | CC BY-SA 3.0 |