ConvertStringToDate: the case of redundant parsing

Xyro

Let's start with the code, verbatim:

    /**
     * The String times are in a format that can not be easily converted to a date
     * variable, so this method parses it and puts it into a format that can be
     * converted to a date variable.  It returns said date variable
     * @param time the String time that is to be parsed and converted to a date variable
     * @return Date the String time converted to a date variable
     */
    public Date convertStringToDate(String time)
    {
        //what the inputed time string will be parsed into: dd/MM/yyyy HHss:SSS
        String dateFormat = "dd/MM/yyyy HHss:SSS";
        String strDate = null;
        //example of what the date comes in as: 20100713_140258_810
        //gets each part of the broker time
        try
        {
            String year = time.substring(0, 4);
            String month = time.substring(4, 6);
            String day = time.substring(6, 8);
            String hour = time.substring(9, 11);
            String minute = time.substring(11, 13);
            String second = time.substring(13, 15);
            String millisecond = time.substring(16, time.length());

            //creates a string containing all the information from the time variable in a usable format
            strDate = day + "/" + month + "/" + year + " " + hour + ":" + minute  + ":" + second  + ":" + millisecond;
        }
        catch(NullPointerException e)
        {
            log.info("String to be converted was null: ", e);

            return null;
        }
        //create a DateFormat instance that will be used to create the date variable
        DateFormat formatter = new SimpleDateFormat(dateFormat);

        Date date = new Date();
        try
        {
            date = formatter.parse(strDate);
        }
        catch (ParseException ex)
        {
            log.info("converting string: " + strDate + " to a date object failed: " + ex);
        }

        return date;
    }

Judging by the consensus, it seems it's bad form not to explain the OP WTF, even if the OP believes it can stand on its own. Yet at the same time, explaining the joke ruins it, but I am not above ruining jokes, so here I go. There are ... multiple WTFs in my coworker's above code, but for now let's just focus on the raison d'être.

The claim is that the incoming datetime format is not easily converted into a Date object, yet the (correct) comment gives an example of "20100713_140258_810" as the input. This can be trivially described as "yyyyMMdd_HHmmss_SSS". In fact, its trivial nature was the reason that format was originally chosen for inter-process (and inter-language) datetime serialization. My coworker is aware of the SimpleDateFormat's parse() method, as well as is aware of the requirement of a format string in its constructor, yet appears to be unaware of the ability for that format string to change. Sigh.

Sigh. I really don't know how to tell him the news. Partly because it's so embarrassing, partly because I wasn't supposed to be poking around in his codebase in the first place. (I was testing a SVN upgrade and his commit comment of "fixed logging issues." caught my eye.) Should I just fix it for him and not say anything? No, that won't work, he wouldn't be update his working copy since he's the only one who works on this project. Should I just let it go? Am I even capable of doing that? Do I have a moral obligation to right this wrong? Ugh, I hate ethical issues.

I hope you've enjoy this brief but all-too-common WTF.

MrDOS

@Xyro said:

Should I just fix it for him and not say anything? No, that won't work, he wouldn't be update his working copy since he's the only one who works on this project.

Maybe he wouldn't immediately notice, but SVN should complain about a conflict next time he tried to check in changes from that file.

Xyro

@Mr. DOS said:

@Xyro said:
Should I just fix it for him and not say anything? No, that won't work, he wouldn't be update his working copy since he's the only one who works on this project.
Maybe he wouldn't immediately notice, but SVN should complain about a conflict next time he tried to check in changes from that file.

That seems so connivish and passive-aggressive though. :(

Lorne Kates

You can just ask him if he knows about DateTime.ParseExact. It could be that he truly doesn't know about it. Just tell him you came across this function while doing some SVN work, and you just wanted to share a tip with him that can save him time and make his life easier in the future. You're not attacking the code, or even saying it's wrong-- just that there's an easier, though maybe not well known way, of doing it. Offer to show him the syntax if he checks out the code (on his workstation, with his hands on the keyboard)

In the end you come off as helpful. You don't have to worry about territorial bullcrap, because he gets to make the change to his code with his SVN account, and has the option not to do it at all. If he does, great, everyone's a winner. If not, you can file that away, give him a point in the "incompetent" column, and tread lightly around anything he checks in.

If you need a CYA, when you offer to show him the syntax, send him an email with "I saw line xxx in SVN, you might want to try DateTime.ParseExact(string, string) instead-- save some time". That way if there is some higher-up code review, no one can blame you for not trying.

MrDOS

@Xyro said:

That seems so connivish and passive-aggressive though. :(

Oh, that's true. I just meant that there's no technical reason to not take that approach.

dhromed

@Xyro said:

I really don't know how to tell him the news.

Fucking hell, just slap his face and tell him.

Power_Troll

@Lorne Kates said:

You can just ask him if he knows about DateTime.ParseExact. It could be that he truly doesn't know about it. Just tell him you came across this function while doing some SVN work, and you just wanted to share a tip with him that can save him time and make his life easier in the future. You're not attacking the code, or even saying it's wrong-- just that there's an easier, though maybe not well known way, of doing it. Offer to show him the syntax if he checks out the code (on his workstation, with his hands on the keyboard)
In the end you come off as helpful. You don't have to worry about territorial bullcrap, because he gets to make the change to his code with his SVN account, and has the option not to do it at all. If he does, great, everyone's a winner. If not, you can file that away, give him a point in the "incompetent" column, and tread lightly around anything he checks in.
If you need a CYA, when you offer to show him the syntax, send him an email with "I saw line xxx in SVN, you might want to try DateTime.ParseExact(string, string) instead-- save some time". That way if there is some higher-up code review, no one can blame you for not trying.

Umm, the code that was posted isn't C#.

blakeyrat

@Power Troll said:

Umm, the code that was posted isn't C#.

It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?

2) If it's Java, or whatever, then why the fuck doesn't Java have an equivalent to ParseExact()? ... also why the hell does anybody use Java for anything ever?

Edit: OH SHIT did I just get trolled by PowerTroll? I hang my head in shame.

boomzilla

@blakeyrat said:

1) It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?

I think the same thing in reverse when C# code gets posted. The main clue is usually String vs string.

@blakeyrat said:

2) If it's Java, or whatever, then why the fuck doesn't ~~Java have~~ the originator of the WTF use an equivalent to ParseExact()?

FTFY. Also, that question kinda answers itself, really.

Jaime

@blakeyrat said:

2) If it's Java, or whatever, then why the fuck doesn't Java have an equivalent to ParseExact()? ... also why the hell does anybody use Java for anything ever?

Java has SimpleDateFormatter which has a parse method that is roughly the same as ParseExact. The whole point of the joke, which the OP even explicitly mentioned, was that the code did use it, but still did a manual conversion to a second format before using it.

Jaime

@boomzilla said:

@blakeyrat said:
1) It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?
I think the same thing in reverse when C# code gets posted. The main clue is usually String vs string.

My main clue was SimpleDateFormatter, which makes your second point irrelevant.

boomzilla

@Jaime said:

@boomzilla said:
@blakeyrat said:
1) It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?

I think the same thing in reverse when C# code gets posted. The main clue is usually String vs string.

My main clue was SimpleDateFormatter, which makes ~~your second point~~ my reading comprehension ~~irrelevant~~ undetected.

FTFY

Jaime

@boomzilla said:

@Jaime said:
@boomzilla said:
@blakeyrat said:
1) It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?
I think the same thing in reverse when C# code gets posted. The main clue is usually String vs string.
My main clue was SimpleDateFormatter, which makes ~~your second point~~ my reading comprehension ~~irrelevant~~ undetected.
FTFY

Really? You modified blakeyrat's line to read:

2) If it's Java, or whatever, then why the fuck doesn't the originator of the WTF use an equivalent to ParseExact()?

Notice the "doesn't". Perhaps sarcasm was supposed to be read into the question?

boomzilla

@Jaime said:

@boomzilla said:
@Jaime said:
@boomzilla said:
@blakeyrat said:
1) It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?

I think the same thing in reverse when C# code gets posted. The main clue is usually String vs string.

My main clue was SimpleDateFormatter, which makes ~~your second point~~ my reading comprehension ~~irrelevant~~ undetected.

FTFY

Really? You modified blakeyrat's line to read:

2) If it's Java, or whatever, then why the fuck doesn't the originator of the WTF use an equivalent to ParseExact()?

Notice the "doesn't". Perhaps sarcasm was supposed to be read into the question?

Uh, yes, I modified that other part, but, um, why does distinguishing between Java and C# code posted to the forum make anything irrelevant about anything else involving the OP?

Lorne Kates

Bah, I'd meant to say "For example, DateTime.ParseExact (or whatever the hell language)".

Point is that the advice is language agnostic. Be helpful rather than a passive agressive dicksocket.

Save being a [s]passive[/s] aggresive dicksocket for the forum.

@boomzilla said:

@blakeyrat said:
1) It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?

I think the same thing in reverse when C# code gets posted. The main clue is usually String vs string.

@blakeyrat said:
2) If it's Java, or whatever, then why the fuck doesn't ~~Java have~~ the originator of the WTF use an equivalent to ParseExact()?

FTFY. Also, that question kinda answers itself, really.

Funnily enough, C# will do either string or String. VB.Net will only do String.

Xyro

@Jaime said:

The whole point of the joke, which the OP even explicitly mentioned

High five!

What I really lament is that I originally wrote the OP mentioning that the language was Java. I later removed the language description in order to make the opening sentence cleaner, and I thought that it wouldn't make that big of a difference anyway. Well, I've learned my lesson.

PS, I'm writing this post in English, specifically the American variant.

The_Assimilator

I would've thought the JavaDoc-style comment, plus the function name starting with a lowercase, plus the fact that C# has no Date object in its BCL, would've made it pretty clear what language we're looking at. Or maybe it just means I've spent too much of my life looking at Java code.

Xyro, how (allegedly) senior is this other developer? If he's been around long enough that he should know how to do this sort of thing in a sane way, fix his code and check it in. If he has any sense, he'll check it out, read over what you've done, realise it's better than his way, and either keep quiet or thank you for the fix. If he blows up at you, politely explain that it is both your and his job to write the best code possible, and since he didn't seem capable of doing that in this instance, you decided to do it for him. That way you look like you're being helpful and he looks like an asshole.

If he's new enough that it's acceptable for him not to know how to do this correctly, fix his code and check it in anyway. Then set aside some time to explain to him what you did, why you did it, and why it's better than what he had. If he's worth keeping he'll listen to you and do it right next time.

Descriptive comments can help a lot (i.e. comments that tell people what to do to fix the code). While waiting for my 10-minute-build to complete, I checked out some of our juniors' code and had a look at it. Where I found things that were bad or ugly, I added appropriate comments, checked the code back in, then asked the original authors to take another look at the code. After they'd had another pass at it I checked it out and went over it again, editing comments where necessary or removing where the fix was acceptable. The ones who didn't make an attempt to fix their code had it fixed by me, but the fixes received comments along the lines of "bad code by <developer> refactored by <me>". I know it isn't nice to name and shame, but I have no patience or sympathy for lazy devs who don't attempt to better themselves.

Power_Troll

@blakeyrat said:

@Power Troll said:
Umm, the code that was posted isn't C#.

It visually parses as C#. I thought it was C# as well. Don't we have a rule yet than posters must mention the language the code sample's in?

2) If it's Java, or whatever, then why the fuck doesn't Java have an equivalent to ParseExact()? ... also why the hell does anybody use Java for anything ever?
Edit: OH SHIT did I just get trolled by PowerTroll? I hang my head in shame.

Actually, you know, you're right - it uses that shitty C style where the leading brace comes underneath the method signature, try, catch, etc. So, I can see how that confused you. Besides, I've never trolled on this forum before, so you're golden.

Weng

@Power Troll said:

Actually, you know, you're right - it uses that shitty C style where the leading brace comes underneath the method signature, try, catch, etc. So, I can see how that confused you. Besides, I've never trolled on this forum before, so you're golden.

Except two sentences right before you wrote that. Unless you're REALLY a godless heretic in need of being burned at the stake.

boh

I would have skipped the parser stuff, and only brought along atoi and a struct SYSTEMTIME to the party.

But then I don't use these newfangled languages either.