• Rob (unregistered)

    We take our newPerc array and turn it back into a string, trimming off any whitespace (which I'm fairly certain whitespace isn't a digit, last I checked, so there's nothing to trim).

    newPerc's length is the same as the input length, but it only gets digits added to it. That means that if the input is 05A, newPerc will contain 05 followed by a \0. The trim will get rid of these trailing \0 characters.

  • Rob (unregistered)

    Note that strPerc.length()!=0 will always be true, because otherwise y would have been 0 and the method would have already returned -1.

  • Greg (unregistered)

    If we disregard the utter nonsense of their validation, it could be that they're abusing the parse code to "signal" whether or not to notify a changed value (model/view done horribly wrong).

  • (nodebb)

    So, it will end up saying that "4.75%" is not a percentage (because it yields the integer 475, which is above 100), while 4.7% yields the integer 47, which it accepts, but is not 4.7...

    But in any event, it doesn't do that, since it doesn't compile. See this line:

          if(percent<;0){
    

    That semicolon will blow up in the compiler.

    Dunno, maybe it's an anonymisation failure.

    Addendum 2025-10-21 10:00: Hmm. 4.75% is rejected because it's longer than 4 characters, not because it's converted to 475.

  • erffrfez (unregistered)

    as this is an override, i'd like to see the original validatePercent please. Maybe it is "better"

  • Wharrgarbl (unregistered)

    This simultaneously makes me laugh and cry at the same time.

  • (nodebb)

    Any function/method with a name beginning "Validate" must return a boolean. Prove me wrong.

  • Captain Obvious (unregistered) in reply to dpm

    I think we already have a counter-example that proves you wrong :)

  • (nodebb)

    Another WTF is that they return Integer, the boxed object type, rather than int, the primitive type, even though all code paths return primitive integer values and they don't need nullability.

  • (nodebb)

    I would expect it to work like this: ... var someNonsense = validatePercent(account.rate, CURRENTRATE); if (someNonsense == -1 ) { ........} ...

    It's a very WTFy way to validate the rate is entered correctly, if this is the use case. I can't think of any other use case, but I'm sure my example isn't what it's trying to do.

  • (nodebb) in reply to dpm

    Any function/method with a name beginning "Validate" must return a boolean. Prove me wrong.

    "prove" could be difficult, but I can justify saying that the return type should, in fact, be void, since the function's name doesn't ask a yes/no question. (Well, it doesn't ask any kind of question...)

    Think of how if ( validateSomeProperty(...) ) reads, compared to if ( isSomePropertyValid(...) ).

  • (nodebb) in reply to Rob

    That means that if the input is 05A, newPerc will contain 05 followed by a \0

    Which \0 is that, in this string that's neither C nor C++, and therefore probably doesn't even have a terminating zero, and in any event, where the length property doesn't include it?

  • Tim R (unregistered) in reply to dpm

    I have often used a convention where "validate..." functions have no return but may throw an exception, whereas the functions that decide whether the input is valid or not and return boolean are called "isValid..."

  • (nodebb) in reply to dpm

    Thanks, now I'm tempted to tuck a "validateThing" into a personal project somewhere that always returns a string. Probably "No" or "I don't wanna."

  • (nodebb)

    \d+(.\d+)?%

    Regular expressions are frequently the correct solution.

  • (nodebb)

    At this point I'm worried that a return value of -1 indicates "success", and anything else is "invalid".

  • (nodebb) in reply to Steve_The_Cynic

    Which \0 is that, in this string that's neither C nor C++,

    This ties in with what I thought when I read the Fine Article, which is either someone got dragged kicking and screaming from C into Java, or a really bad Java programmer found a C example (to do something) and changed it enough to make it at least compile in Java.

  • mihi (unregistered) in reply to Steve_The_Cynic

    Not the string. The array gets initialized with null bytes (as many as there are characters in the string)...

  • Alan Scrivener (unregistered)

    Never have a seen a function more sorely in need of a spec.

  • (nodebb) in reply to dpm

    Any function/method with a name beginning "Validate" must return a boolean. Prove me wrong.

    I think you confused a validator with a parser.

    A parser returns result and signals the success of the operation. In a lot of OOP languages the signal is an exception with often an additional exception-less method, for functional languages it's an union with an status code (which is often a boolean). In Java it's a mess.

    A validator returns a collection of property groups which broken validation rules. It works the same in most languages. Obviously it's again a mess in Java.

    The difference for a software design perspective is that a parser fails as soon as an error occurs but a validator validates every single rule, so you don't have to iterate over the the check blindly which is important for both UX and non-trivial remote operations like for web services.

    And it's a mess in Java because while a lot languages offer a standardized solution how to implement both or at least best practices, there is not such thing in Java and every one seems to implement his own thing in a glorious mess.

    Addendum 2025-10-22 01:41: Oh, I forgot an important other aspect where you want to use a validator over an parser and that is validation of security credentials. There reason is simply because otherwise you open yourself up to timing-attacks because the cracker can start guessing where the validation fails and this would exponentially weaken your authorization. Hence you should also always use full comparisons for secrets and not first-error parsers.

Leave a comment on “A Percentage of Refactoring”

Log In or post as a guest

Replying to comment #685680:

« Return to Article