• (nodebb)

    Frist off, it seems pretty clear this code got through every review it was exposed to. All zero of them.

  • Hanzito (unregistered)

    Frist off... now there's a nice invective!

    Frist off, ++!

  • (nodebb) in reply to WTFGuy

    I admire the happy world you live in, where code reviews are actually meaningful and people care about non-optimal code --- it works, it gets approved. No one cares if it could be improved, there's a new sprint to worry about.

  • Michael S. (unregistered)

    Don't forget that the original coder forgot to release resources in a finally block. I bet you they have IO issues due to the application keeping so many files open.

  • (nodebb)

    They also didn't pass a character encoding parameter into FileReader, which means it's using their system's default encoding, whatever that is. To be fair, that'll probably always work for reading urls, but why rely on a "probably" when you could easily have a "definitely"?

  • (nodebb) in reply to WTFGuy

    Frist off, it seems pretty clear this code got through every review it was exposed to. All zero of them.

    That's "zero++" to you, sir.

  • (nodebb)

    I'd bet this code predates Java 8 significantly - the choice of using a Vector over and ArrayList and the use of the Enumeration type instead of Iterator gives a whiff that at least the tutorial or book they were using predates Java 1.2

  • (nodebb)

    @Balor64 ref

    They also didn't pass a character encoding parameter into FileReader, which means it's using their system's default encoding, whatever that is. To be fair, that'll probably always work for reading urls, but why rely on a "probably" when you could easily have a "definitely"?

    Unless they also explicitly control the encoding used by whatever app & process to write that source file, choosing a specific encoding is no more "definitely" successful than is the system default.

    if we want to assume the file is created on that same server by some process, leaving the encoding unspecified is probably the safer bet. Doubly so if this is software that's sold to multiple customers who might be anywhere on Earth. We'd like to believe Unicode solved all that, but it didn't completely.

  • Duke of New York (unregistered)

    Where we're going, we don't need close().

  • (nodebb) in reply to WTFGuy

    leaving the encoding unspecified is probably the safer bet.

    I think I'd prefer it if they explicitly specified Charset.defaultCharset() to prove that this was their definite intent and that they hadn't just forgotten about it.

  • MRAB (unregistered)

    Why did they count the records with recCnt? Was it because they didn't know that there's a method for that?

    Oh, wait a moment.

    I see vURLs.size() later on...

  • (nodebb) in reply to dpm
    1. I would reject the code during reviews
    2. I would to to their manager and see if we can get this one fired ASAP

Leave a comment on “Buff Reading”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article