• (disco)

    Could be worse. It could have forced you to put the cabinets back in the same order you found them 14 hours later.

  • (disco)

    Oh, and that exception handler is one step up from an empty catch block, and a small step at that.

    } catch (Exception e) {
   log.error(e.toString(), e);
}

    Well, if the list was never correctly sorted in the first place, what could still happen at that point? At least the exception gets logged so that you know what blew up where.

    I'm more concerned about it returning null when an exception happens...

  • (disco) in reply to JBert

    Logged twice, actually...

  • (disco) in reply to sloosecannon

    How do you mean? Log4J uses the first argument as a message (which could contain more info about the context), the second argument is for printing the exception and it's stacktrace.

  • (disco)

    Is this code implying that it iterates over the cabinets to individually add said cabinets (as cabinets of another (simpler?) type) to the list to sort them (sans last element in the list)?

    Maybe the requirements was to create separation between the Ajax side and the application?

  • (disco) in reply to JBert

    The e.toString() gives some data about the exception, which is then printed again when the stack trace is printed. It's not technically logged twice but the e.toString() is pretty much useless

  • (disco)

    INB4 listed The article is already visible on the frontpage.

  • (disco)

    Well the implementation leaves some room for improvement, but overall we can see the programmer was careful to write robust code. For example, the repeated sorting ensures the list is never left in an unsorted state. So even when it'd abort on an exception, it could return the list it built so far and it would be mostly sorted. I think keeping order is best done continuously so you never have a big mess at hands!

  • (disco) in reply to gleemonk
    gleemonk:
    ell the implementation leaves some room for improvement, but overall we can see the programmer was careful to write robust code. For example, the repeated sorting ensures the list is never left in an unsorted state. So even when it'd abort on an exception, it could return the list it built so far and it would be mostly sorted. I think keeping order is best done continuously so you never have a big mess at hands!

    and you always know which element was most recently added -- it's the last one - bonus!!

  • (disco) 
            } finally {
            HibernateSessionFactory.closeSession();
        }

    is only executed if something goes wrong (return in the try block).

    But no matter what - anything with an open or a close in it without a corresponding close resp. open in the same procedure and called under the exactly same conditions (save for the corresponding open having failed) looks so wrong to me it hurts.

  • (disco) in reply to RFoxmich
    RFoxmich:
    _and_ you always know which element was most recently added -- it's the last one - bonus!!

    Nice feature! I think programmers these days are too obsessed with efficiency. For starters, this article doesn't even measure the performance but relies on some academic notion of how performant it ought to be.

    Well in theory the code could run in O(n) if the result of getAllPrograms() was already sorted and Collections.sort() could introspect what parts of a Vector were already sorted. Eat that pessimists.

  • (disco)
  • (disco) in reply to PWolff

    a finally clause is always executed, regardless of return statements or exceptions.

  • (disco) in reply to negatyv

    Right. Had forgotten. Confusing, though.

    (I avoid that kind of pattern if possible. Am fine with using, though.)

    That doesn't change anything about the fact that there is only one place here that mentions HibernateSessionFactory.

  • (disco) in reply to PWolff

    Mmm nope...

    Typically, the statements of a finally block run when control leaves a try statement. The transfer of control can occur as a result of normal execution, of execution of a break, continue, goto, or return statement, or of propagation of an exception out of the try statement.

    Source: https://msdn.microsoft.com/en-us/library/zwc8s4fz.aspx

  • (disco)

    Sorting on every insert. Very common mistake. One line fix.

  • (disco)

    Also, use the Vector class for some additional thread-safe inefficiency.

  • (disco) in reply to CRi

    Umm, isn't this Java code? Because in Java, the finally would be executed.

  • (disco) in reply to CRi
    CRi:
    > The transfer of control can occur as a result of normal execution, of execution of a break, continue, goto, or return statement, or of propagation of an exception out of the try statement.

    Surprising yet sane behavior.

  • (disco) in reply to BobbyTables

    It is Java... man I need more coffee today :sweat_smile:

    Anyway, in C#, the finally is executed in all cases too.

  • (disco) in reply to negatyv

    The other problem: if whatever goes wrong causes the VM to be hosed, then the finally clause won't be run because the process that was supposed to run it is sitting in a corner crying.

  • (disco)

    Um, so TRWTF is "while it.hasNext()" instead of "for (Iterator it = cabinets.iterator(); it.hasNext(); it.next())" or whatever the syntax is that I can't be bothered to check?

    Of course, what David S. should immediately do is use:

    	for (SearchProgramShell cabinet : SpecificObjectManager.getAllPrograms())

    and so replace 4 lines of code with 1. A significant improvement.

    Oh come on. Nobody is using pre-Java-5 any more.

  • (disco) in reply to Matt_Westwood

    There is exactly one reason to use the Iterator syntax that I can think of:

    For a lot of Collection types, the Iterator's remove command is the only way to remove an element while iterating without Java throwing a ConcurrentModificationException.

  • (disco)

    I would think the order would be something like...

    On2 log n

    Except that, since it doesn't sort the last element added each time, maybe...

    O(n2-n) log n

    ...and that is terrible. A bubble sort would be faster. You can at least tweak that to get O½n2.

  • (disco)
    Remy Porter:
    I leave the total Big-O for this implementation up to the reader.
    I calculate this as being O(MG!)
  • (disco) in reply to dkf

    Meanwhile, George Takei calculates this as being O(My)!

  • (disco)

    Although this is terrible, interestingly enough in Java 7 (and up) the overall O is lower, thanks to TimSort and its huge bag of special cases.

  • (disco) in reply to Placeholder

    What we have here is a failure to communicate.

  • (disco) in reply to cellocgw
    cellocgw:
    What we have here is a failure to communicate.

    YMBNH

    It's curious that the cabinet reference went unseen by you, what do they teach at university these days? The world of disks that we call the internet has more than enough information in it for you to figure out what I am talking about.

  • (disco) in reply to Placeholder

    Actually initially when I saw the 'SearchProgramShell' thing my brain parsed that as 'shell: sort list' (that is, fork a shell, call /bin/sort with a list of elements and parse the result back in again...., Yes that is possible to do in some languages, however I would never recommend that....)

  • (disco) in reply to Placeholder

    Maybe you're one of those young 'uns messing up my lawn. The "empty and then replace later" is highly reminiscent of "Cool Hand Luke." Hence my response.

    PS I've been on TDWTF for at least 8 yrs. how 'bout you :smile:

  • (disco) in reply to cellocgw
    cellocgw:
    The "empty and then replace later" is highly reminiscent of "Cool Hand Luke." Hence my response.

    The "14 hours later" part was supposed to be a reference to a cabinet in the Discworld novels. Hence my response.

    cellocgw:
    PS I've been on TDWTF for at least 8 yrs.
    For all I know, based on your post and likes history, you never got past the articles section :smile:.
  • (disco) in reply to cellocgw
    cellocgw:
    empty and then replace later
    Reminds me of JavaSpaces...
  • (disco) in reply to CoyneTheDup

    I think code like this is why Apple's Objective-C sort first checks whether there is a sequence sorted in ascending / descending order at the start and the end of an array. So when you add the n-th element and sort, it sees in O (n) that the first n-1 elements are sorted, and can merge the subarray from 1 to n-1 with the subarray from n to n, also in linear time. All in all it's not much slower than a linear insertion sort. O (n^2).

    (Seriously, it happens quite often that an array is sorted which is already mostly sorted).

  • (disco) in reply to PWolff

    @PWolff The whole point of "finally" is that it always executes no matter what happened before.

  • (disco) in reply to BobbyTables

    And Delphi

  • (disco)

    Clearly the goal of this code is to optimise the m.add(ca) by giving it a pre-sorted list. :passport_control:

    Of course that would only make sense if m.add() needed a sorted list in the first place, which it would only do if it was adding the new element in the correct place, in which case you wouldn't need to sort the list explicitly because it would already be sorted. But hey, defensive coding, right?

  • (disco)

    Was this before the introduction of sorted lists? Otherwise that guy might have used one, and write a hand-crafted routine to verify it was indeed sorted. (And sort it, should it be unsorted.)

    Anyone else that wants to tell that a finally block will be executed no matter what (unless the thread is killed?)

Leave a comment on “Sorting Cabinets”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article