|
This[^] is a great article imo on the fundamental flaw of SRP we've all seen via arguments at code reviews. It's much more productive to think in terms of cohesion and coupling. In this example validation and parsing are very cohesive which others have pointed out.
All you'd be doing by splitting the method is forcing boilerplate client code (if(can()) then do();) and forcing your own do() method to throw an exception in case of failure where before that wasn't necessary since a validate was in the pipeline already. Considering parsing is an operation that can fail quite often, throwing exceptions isn't an optimal design due to their cost.
|
|
|
|
|
Splitting it in two methods would be a performance hit.
TryParse will need to evaluate the entire string (and maybe actually parse it) to then return true or false, then Parse will do everything again.
I say actually parse it because for ints (and many other types, actually) the string length and characters might be valid, yet the value overflows and the easiest way for a TryParse to detect it is to actually do the full parsing process.
That being said, most TryParse methods today would be best implemented by using a Nullable<t> (or simply T?). Unfortunately, this will only work for structs (but it would also be valid for classes if a parse is never expected to return null).
|
|
|
|
|
What we have here is a case of pragmatism vs. principle.
TryParse is "wrong" in many ways - violation of SRP, use of exceptions as a method of flow control and so forth - but the alternatives are not too hot, either.
Whilst I maintain, contrary to what a lot have suggested, that conversion values could be validated without actually parsing, to do so would involve some mile-long regular expressions (if we take integer as the most easily parsed type, even a ranged regex for that would be quite a hefty beast).
The consensus seems to be that MS have taken the right approach here by taking the pragmatic route and I tend to agree with that but I can't help wondering how many of us would flag this up in a code review.
It would also be interesting to know how those who voted that outs should never be used would approach the problem.
98.4% of statistics are made up on the spot.
|
|
|
|
|
TryParse doesn't use exceptions as a method of flow control. Parse does. TryParse returns a boolean (the exception is only for null values, if I'm not wrong).
About validating with regular expressions, it would be actually much slower than doing the actual TryParse, and that means pre-validating to do a parse will be slower than the entire operation.
In the end, what happens is that now we have more alternatives than out parameters. For ints, for example, using int? (that is, Nullable<int>) would do the job pretty well. I never really tried to check the performance difference to see if there's any, but it looks nicer than the out parameter.
|
|
|
|
|
PeejayAdams wrote: TryParse is "wrong" in many ways - violation of SRP, use of exceptions as a method of flow control and so forth
Your comments may be driven by a misunderstanding of how TryParse works under the covers. The "Try" in the name doesn't refer to the "try" statement for catching exceptions. The method parses as it goes and if it comes to a character it can't parse it returns false, so you could argue (tenuously I admit) that TryParse doesn't validate, it only parses. The alternatives are to split the methods out which means doing the validation aspect twice. Most people would agree that breaking SRP (if it does) for such a trivial and oft-used method is a great price to pay for the performance gained. You shouldn't adhere to principals so strongly that they have negative effects elsewhere. Yes SOLID is good, but don't throw the baby out with the bathwater.
|
|
|
|
|
PeejayAdams wrote: which violate the SRP by both validating and parsing.
Yes, and CQS - Command Query Separation, which I find makes for cleaner and easier to understand code.
Kevin
|
|
|
|
|
|
They used to be useful, but they will become less useful in C++17.
Structured bindings make it much easier to have multiple return values.
Enjoy life, this is not a rehearsal !!!
|
|
|
|
|
with salt and lime!
Don't let your mind wander too far.
It's too small to be let out alone.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
