Timeline for Are too many if-else statements for validation bad?
Current License: CC BY-SA 3.0
13 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Aug 30, 2013 at 4:46 | comment | added | iAteABug_And_iLiked_it | @itsbruce "Writing code as presented in this snipped from the book is nearly as much work as writing better code in the first place and generates much more work in the long run" ... I echo your thoughts | |
| Aug 29, 2013 at 17:06 | comment | added | Andy | @Telastyn "Default construct the object to satisfy the rules and modify it to break the one you're testing." That's actually a pretty terrible way to do things, especially in .Net. It means you cannot make any use of MS' various data binding technologies (which they have in all of their UIs). | |
| Aug 29, 2013 at 15:30 | comment | added | itsbruce | @JimmyHoffa "validation" is the wrong term and seems only to be confusing people. These are rules to affect the flow of a business process. A borrower object which fails them is not an invalid object, just a person doomed to disappointment. | |
| Aug 29, 2013 at 14:45 | comment | added | itsbruce | @Telastyn " Default construct the object to satisfy the rules and modify it to break the one you're testing." Changing the default test-construction of the user is rewriting the test suite, or are you really suggesting that the borrower object's actual constructor generate perfect users by default? Writing code as presented in this snipped from the book is nearly as much work as writing better code in the first place and generates much more work in the long run. | |
| Aug 29, 2013 at 14:40 | comment | added | Jimmy Hoffa | I'm not that bright; I missed that this was a model with validation, I read the code snippet as an operational method that validated it's arguments. +1 this is the right approach. | |
| Aug 29, 2013 at 14:34 | comment | added | itsbruce | @iAteABug_And_iLiked_it you can always argue with rep. Simply accepting an argument because of rep is the "Argument from authority" logical fallacy (en.wikipedia.org/wiki/Argument_from_authority). Aristotle had (and still has) huge rep but that doesn't mean that vultures are impregnated by the wind. Not saying Aristotle was a rep-whore, of course ;) | |
| Aug 29, 2013 at 14:33 | comment | added | Telastyn | @itsbruce - In practice, I find your #3 to be troublesome/impractical. I also don't see how you're rewriting testing logic for a business rule. Default construct the object to satisfy the rules and modify it to break the one you're testing. I do agree that having validators inspect the object like you describe is nicer, cleaner and more extensible. It's very likely how I would do it if presented the problem in a vacuum, but I also probably wouldn't bother refactoring the book's code if I ran into it. | |
| Aug 29, 2013 at 14:23 | comment | added | itsbruce | I disagree with you completely. Decoupling this is quite easy and makes the code significantly simpler. Explanation below. The implementation in the snippet is not easy to test at all, since the rules are all hidden inside a method and not properly defined anywhere. You'd have to rewrite the testing logic for Borrower every time you added a new business rule, which is wrong. | |
| Aug 29, 2013 at 13:52 | comment | added | Daniel B | While I agree that this is pretty standard and possibly good enough (in the "we've got bigger problems elsewhere" kind of way), it's still not particularly pretty. It's not DRY (e.g. one or two classes down the line I'd get irritated that child object's validations have to be manually merged in, with the same code each time), encourages monolithic methods (with high cyclomatic complexity), has a low signal to noise ratio, etc. To be fair, the book might later on refactor some of it into a more generic form. | |
| Aug 29, 2013 at 13:32 | comment | added | iAteABug_And_iLiked_it | Well said! just that finding this kind of code in a blog is fine, but in a book which teaches decoupled design, its just...silly | |
| Aug 29, 2013 at 13:30 | comment | added | Telastyn | @iAteABug_And_iLiked_it - No, by all means you should argue with me if you think I'm wrong. And in general, you should go nuts with decoupling - but there's always some line where you need to stop or else you end up with classes that have only one variable making your code completely unusable. That line is a trade off, and one you'll be able to guess more accurately with experience. If you're light on experience, err towards more decoupling. | |
| Aug 29, 2013 at 13:22 | comment | added | iAteABug_And_iLiked_it | Can't argue with 22.8K rep... My issue was that first we are told to decouple, decouple, decouple, and separate validation logic from data model and then you see this kind of code from people with years of experience doing the opposite and you just get so confused | |
| Aug 29, 2013 at 13:08 | history | answered | Telastyn | CC BY-SA 3.0 |