• bvs23bkv33 (unregistered)

    true, we found first comment

  • (nodebb)

    true Best code WTF in a while.

  • (nodebb) in reply to bvs23bkv33

    true, "That is the answer"

    Addendum 2016-09-20 06:57: Oh, forgot to add out to the parameters

  • DBarb (unregistered)

    You know what this is? This is the result of a manager coding drive-by: approaches glances at monitor Make it more enterprise-y! continues past

  • me (unregistered)

    Maybe the coding guidelines forbid multiple return statements in one function and breaks in a for loop and poor developer simply didn't see another way to shortcut the loop.

  • Discouraged (unregistered) in reply to me

    Oh, I think there's definitely a poor developer involved here, but not in the way you suggest...

  • Rich (github)

    I assume this was conceived so that it returns false in the case of another exception, for example: a permissions error.

    Which would be just about OK until one day you try to read a file called "true_lies.groovy" and don't have permission...

  • Koen van Dratel (unregistered)

    I only know about Groovy what I just read at "http://docs.groovy-lang.org/latest/html/groovy-jdk/java/io/Reader.html#eachLine(int, groovy.lang.Closure)". If that info is complete, throwing an exception from within the callback is the only way to avoid scanning entire files, which might be a concern. Yes, the code can be made a bit nicer and a lot more robust, but the TWTF seems to be the API.

  • Hanzito (unregistered) in reply to Koen van Dratel

    What you might have been looking for was headerFile.someLine(fun). If that doesn't exist, !headerFile.eachLine(!fun) will do the same.

  • TheCPUWizard (unregistered)

    Koen van Dratel got it (and it seems others missed it...)... One of the problems with iterators that take simple delegates/lambdas is that an exception is the only way to terminate the iteration. Given that constraint the try/catch is necessary. So the other exception, while not ideal has some advantages and few real problems...

  • dpm (unregistered)

    then at least avoid testing a string!

    bool found = false;
    File headerFile = new File(headerPath)
    try {
        headerFile.eachLine {
            if (it.contains("MODULE_INCLUDE core.") ||
                    it.contains("import core.")) {
                         found = true;
                        throw new Exception("true, we found a good import")
                    }
             }
        }
        throw new Exception("false, no good import found")
    } catch (Exception e) {
        return found;
    }
    
  • Confused (unregistered)

    Why does clicking on the second "Transpiler" word create.... that?

  • Foo AKA Fooo (unregistered) in reply to TheCPUWizard

    I know just as much about Groovy as Koen van Dratel does. If there are no alternatives (like Hanzito suggested) available, the API is TWTF. But TRWTF is still using the exception message to decide the result. One could at least use a special exception class and catch that specifically. Even if it means having to define that class just for this purpose, IMHO that's mandatory if 'e.getMessage().contains("true")' is the alternative.

  • Koen van Dratel (unregistered) in reply to Hanzito

    Actually I did waste a couple of seconds on locating sth. that could be named someLine. I'm sure that if it exists it doesn't want to be found. Your alternative that "does the same" is actually quite funny ;-)

  • Koen van Dratel (unregistered) in reply to Foo AKA Fooo

    I agree, make a unique subclass of Exception to avoid having to inspect the generic message and getting false positives. Better yet, don't use that API for this task, or reconsider whether it's really a problem to scan each file in full.

  • Angela Anuszewski (google) in reply to Confused

    It is a long-running gag in Remy's articles. There are also comments usually embedded in Remy's articles, so be sure to view the HTML source as well.

  • (nodebb)

    throw new Exception("false, found nothing that could me misconstrued as an import.");

    Addendum 2016-09-20 10:40: The "mis-" might be superfluous, BTW.

  • balazs (unregistered) in reply to dpm

    This line: throw new Exception("false, no good import found") is outside of the eachLine callback, so just replace with "return false"

    And replace the Exception with a special subclass (e.g. ModuleFoundException), throw and catch that to avoid messing with any natural exception that can occur in the process.

  • Søren Berg Glasius (github)

    This would work as well, and would not need an exception:

    private static boolean shouldConvert(String headerPath) {
        File headerFile = new File(headerPath)
       
        def writable = headerFile.filterLine {it.contains("MODULE_INCLUDE core.") || it.contains("import core.")}
        return writable.toString() // If the string length is > 0 then lines above are included
                
    }
    
  • Herby (unregistered)

    I've always wondered a bunch of things about programs. For this one, I might ask the question "How would I do it using Fortran 66". Then take it as a challenge.

    So today's exercise is.......

    //FORT.SYSIN dd * <<keep calm and carry on.>>

  • airdrik (unregistered)

    While the Reader interface doesn't have what you want directly, it does have an iterator method and Iterator was extended with an any method which does what you expect. So, you can do file.iterator().any { <predicate> } and get the job done without using exceptions for control flow.

  • Developer Dude (unregistered)

    Yes, we have no bananas today

    No - I am a robot!

  • isthisunique (unregistered)

    This code is pretty fubar mainly because scraping an exception message for information is not the way to go. The message is for a human. Throwing an exception within a try is also 999 times out of 1000 wrong.

    However personally I tend to find exceptions poorly defined. There's no notion sometimes of how to handle them. You sometimes have exceptions extending things but I always find that's not enough. I often end up creating my own set of exceptions.

    The first thing you have to think is who or what are the messages for? Is the message for the developer, log, user so that they can correct something, etc?

    What about nested errors or exceptions? Personally, I avoid that.

    I then always find interface exceptions at the top level are useful for highly modular reusable code. For example, if the user is not logged in, then the exception ideally wants to be one that indicates the course of action is to return to the login page. If the user tries to buy a product but the price has changed because a special offer expired, the exception might be a special type of exception that also included the new price and offer details so the interface can be updated and the user can try again. Things like retry exceptions might be a bit dangerous but occasionally when carefully applied make sense. With external sources that have some error rate above 0 then it helps to know that the exception is not necessarily absolute.

    I've done quite a few things with these kinds of exceptions that basically indicate what, if anything, can be done once they reach the IO layer or controlling loop (for example, external source failed, ok to use cache this time and retry later, controlling loop may then impose an acceptable duration for failures). It would be interesting to know if anyone else has done anything like this.

  • foxyshadis (unregistered) in reply to Koen van Dratel

    You can only break out with an exception because you really shouldn't be combining functional programming with procedural. each/eachline and closures are meant to process a collection as a whole, and crowbarring old paradigms into that misses the point completely. It's not a bad API design, they're just trying to make it do something that it was never meant to do, just because it's familiar. When your only tool is a hammer, etc.

    At best, you could argue that Groovy should expand its library to include that common idiom, as well.

    The correct solution is demonstrated on StackOverflow: https://stackoverflow.com/a/214443/321935 Create a near-identical method that does what you actually need. Although this answer doesn't go into it, since this is Groovy you could easily just monkeypatch it into File directly and not need using(FileHelper) { }. (Argument over the merits of monkeypatching a standard API to the left.)

  • jgh (unregistered)

    Transpiler? Groovy? WTF have kids today done to software engineering?

    Gerroff my lawn!

  • Norman Diamond (unregistered)

    So today's exercise is.......

    //FORT.SYSIN dd * <<keep calm="" and="" carry="" on.="">>

    </keep>

    That would be something like

    cc <%lt;/*

    ...

    /*

    but that wouldn't work because the cc command reads disk files not stdin.

    If you want to read data at runtime, you need

    //GO.SYSIN DD *

  • Norman Diamond (unregistered)

    Throwing an exception within a try is also 999 times out of 1000 wrong.

    How do you detect the exception 1 time out of 1000? Throw it?

    However personally I tend to find exceptions poorly defined.

    A few weren't, but when they were found someone threw them.

    On error resume next.

  • drkstr101 (unregistered) in reply to Koen van Dratel

    There are special built in iterator and collectors functions which support early termination.

  • Koen van Dratel (unregistered) in reply to foxyshadis

    You're saying a couple of sensible things, such as that "each" is meant for each line so don't abuse it for sth. else, and about applying a hammer to a screw. My hammer would be "NotAnException extends Exception" but that kind of logic would give fits to Hanzito, so I'd redesign it as "extends Throwable". What's your point?

  • progger (unregistered)

    Genius. I'm now in the process of porting shouldConvert() to Python for the purposes of concealing it all throughout one of the gargantuan legacy Python apps I have the privilege of being the maintainer for.

    Happy "Hide the Nugget" everybody!

  • Carl (google)

    Now all that's missing is an exception called "this exception is false" with a bug report to say it isn't handled correctly.

Leave a comment on “Exceptional Condition”

Log In or post as a guest

Replying to comment #468316:

« Return to Article