Defensive programming with IF+Enumerator statements

60 views
Skip to first unread message

Israel Fonseca

unread,
Jul 28, 2016, 5:00:14 PM7/28/16
to clean-code...@googlegroups.com
Given the following pseudo-code:

```
status = third_party_gateway.do_billing(billing_details)
if status == CONFIRMED
    [code in here]
else:
    [code in here]
```
vs

```
status = third_party_gateway.do_billing(billing_details)
if status == CONFIRMED
    [code in here]
elif status == REPROVED:
    [code in here]
else:
    raise ProgrammingError('Something nasty has happen')
```
The second form can avoid/notify of a defect that would occur if more status were implemented later. Isn't the second form always better? Or should I look for a psychiatrist to treat my paranoia?

Disclaimer: You can't change the third_party code.

Caio Fernando Bertoldi Paes de Andrade

unread,
Jul 28, 2016, 5:49:57 PM7/28/16
to clean-code...@googlegroups.com
This design would be much better if you could encapsulate the actions inside the status themselves:

```
smart_billing_object_based_on_billing_details = my_custom_gateway_that_talks_to_the_third_party_stuff(billing_details)
smart_billing_object_based_on_billing_details.do_something()
```

And you have polymorphism decide what to do, based on which Status you have/are.
This has the extended benefit of now having to worry about a different/new status, since you’ll have to manually support it, your main code there is protected from that change.

Caio

--
The only way to go fast is to go well.
---
You received this message because you are subscribed to the Google Groups "Clean Code Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clean-code-discu...@googlegroups.com.
To post to this group, send email to clean-code...@googlegroups.com.
Visit this group at https://groups.google.com/group/clean-code-discussion.

Antônio Carvalho Jr.

unread,
Jul 28, 2016, 10:23:46 PM7/28/16
to clean-code...@googlegroups.com
If you can, and it is doable, definitely go with Caio's suggestion.

But, if for any reason (other than "I don't know", because in this case you should go learn it) you can't use his proposed solution, consider the questions:


A) How likely is it for the system to have another `status` added?

B) Who handles that code? If a team, how good is that team?


If (A) is highly unlikely, then I'd use option one (without the exception). Just because if it changed it would probably be a big thing and people should take care anyway.

But... if (A) is very likely or (B) is a team, and its members are average (or communication is bad), then I'd just have the exception there.


Thinking out of the box, if `status` is something like an enum, another option could be using option one (w/o the exception) and creating a test (nearby the tests of option one's module) that checks if the status has no elements other than CONFIRMED or REPROVED.



That's all I can think of, anyway.
Simple and complicated, eh? It seems we all need a shrink... :)

Israel Fonseca

unread,
Jul 29, 2016, 12:58:38 PM7/29/16
to clean-code...@googlegroups.com
@caio, in your approach I don't get it how the client would define the behavior based on the outcome. The third_party library shouldn't and wouldn't know which implementation of the 'success' and 'failure' scenario to return. In your example it seemed to me that the gateway could just call the 'do_something' function by itself without returning it to the client. Another alternative would the client send callback functions for the two scenarios. 

Anyway, somewhere a `if` would still exists, and my first question would still applies.

@antônio, you seems to prefer to raise the exception only as last resource, why is that? Is it real worse for the code to raise an exception in a unexpected cause? What are we losing by raising an exception in those edge situations in the border of our apps?


Antônio Carvalho Jr.

unread,
Jul 30, 2016, 3:29:31 PM7/30/16
to clean-code...@googlegroups.com
I don't actually see it as last resort, I'd just only throw the exception when needed, really.

Hope this is not too abstract, or too obvious, but if you think every code shows intention, the presence of the exception tells me the programmer was worried about that possibility at the time.

"Just in case" is a fair reason, but a lot of times that kind of code is just noise...

Imagine there are only two possible status codes and no foreseeable chance of the creation of a new one, but the exception is still added. Time goes on and, 2 years later, a new programmer comes by and finds that exception. Either s/he would just ignore (which is a bad behavior -- see "cargo cult programming"), or would be thinking "why is this here? AFAIK there are no other possible status codes... Am I missing something?!", which would have them wasting a lot of energy worrying about that code (maybe searching for the "other" possible status).

Of course, one could say I'm exaggerating, but I hope you see my point.


[]s
acdcjunior


Reply all
Reply to author
Forward
0 new messages