• Naomi (unregistered)

    Mucking up your instance variables with m_ and explicitly using this to access them? That's a paddlin'.

  • (nodebb)

    Doesn't this have multiple rollover problems, given all the individual Datetime.Now calls ? i.e. turning a time component from 12:59:59 into 12:59:00 because by the time it got to the seconds component it was 13:00:00 ? Oh plus it's compounded due to the highlighted weird padding method also having such calls throughout.

  • (nodebb)

    The fun thing about this code is that sometimes, it generates wrong padding. I fully expect that in their bug tracker, there is at least oen "strange file name of irregular length has been generated" bug closed as CANNOT REPRODUCE.

  • (nodebb)

    I sometimes wonder, how would you fix such a beast? is it even fixable? would corporate allow one to fix it, or just "wrap a band-aid" around it because changing it might have unidentified risks?

  • Hanzito (unregistered)

    You've got to admire adding a constant for the padding character '0' and giving it the informative name c0. It saves one character every time you refer to it, so it has that going for it. And imagine being able to replace it with '1' consistently!

  • (nodebb) in reply to Domin Abbus

    The first fix, to resolve the occasional (and almost impossible to reliably reproduce) glitches noted by @Athanasius and @Kamil, would be to capture Datetime.Now into a variable and use that in all the formatting stuff. Doing that would leave the overall structure alone, which would go a long way to alleviating the "don't mess with it because it works" anxiety.

  • (nodebb)

    Ah, let's play "Spot the clueless dev with ease":

    (1) Using 'char' instead of 'var'.

    (2) Using 'this'. Honorable mention: The 'm_' prefix for private members is actually .net 1.0 style conform, so the rest of the type was actually done by someone old school, but competent.

    (3) Using DateTime.Now multiple times instead of locally caching it.

    (3) Using type over keyword ('String' instead of 'string').

    (4) Using multiple string concatinations over string.Format (or these days $).

    (5) Not using DateTime.ToString(string format, CultureInfo culture);

    m_FileName = $"{DateTime.Now:yyyyMMddHHmmss}_{publication.Bipad:D5}.dat"; // .net 5+

m_FileName = string.Format("{0:yyyyMMddHHmmss}_{1:D5}.dat", DateTime.Now, publication.Bipad); // .net 1.0+
  • (nodebb)

    It's been a minute since some bad date handling code.

    I see what you did there.

  • (nodebb) in reply to Steve_The_Cynic

    Beside the fact that it doesn't work, it's also super expensive. DateTime.Now is actually not only ending up as an unmanaged system call that needs mashalling, it also have to convert the timestamp to the local timezone which could end up in another unmanaged system call that needs mashalling. I wouldn't be surprised if that thing ends up running 10x longer than a proper implementation. That's some impressive display of incompetence for a code that is properly implemented one line.

    Addendum 2024-02-12 07:34: https://referencesource.microsoft.com/#mscorlib/system/datetime.cs

    For those interested how it actually works across all platforms.

  • (nodebb) in reply to Domin Abbus

    I know one developer that writes code like this and I'm always wondering why he hasn't been fired. The answer is: the code works and performance is not a concern so move on. They don't mind if I change the code to make it readable as long as I don't have something else to do, but that's rarely the case.

    One time I had to fix a bug in their code and after the third attempt at undestanding what they coded I just gave up. I had the specifications so I knew what the method was supposed to do but that's as far as I got. I ended up coding the method from scratch and the 100+ lines of spaguetti copy-pasta turned into 20+ lines of actually readable code that also happened to work as specified.

    If I wasn't under NDA, the TDWTF would have banned me by now for spamming them with CodeSODs.

  • Sauron (unregistered) in reply to Domin Abbus

    http://thecodelesscode.com/case/33

    You don't fix such a beast. You bulldozer such a beast, then you build something sane in its stead.

  • Deeseearr (unregistered) in reply to Athanasius

    turning a time component from 12:59:59 into 12:59:00 because by the time it got to the seconds component it was 13:00:00

    Oh, it's even better than that. This code could quite happily tell you that it's 20230101000000 on January 1st 2024... Assuming that the compiler handles string concatenation that way. It could also evaluate everything from right to left, or just take a visit to nasal demon country and start evaluating from the middle. It's the gift that just keeps on giving.

  • Sou Eu (unregistered) in reply to Domin Abbus

    Admittedly I don't program in C#, but I'd do what even the article mentions: replace the innards of the function with a dateformatter and then do whatever that bipad part does.

  • (nodebb) in reply to Deeseearr

    It could also evaluate everything from right to left, or just take a visit to nasal demon country and start evaluating from the middle.

    No. It's C#, and the order of evaluation inside complex expressions is specified far more tightly than in C and C++.

  • Duke of New York (unregistered) in reply to MaxiTB

    The second (3) is pedantry. (4) is also pedantry, contradicts Microsoft's guidance ("How to concatenate multiple strings") and suggests a severely antiquated understanding of how concatenation expressions are compiled (see LocalRewriter_StringConcat.cs).

  • Duke of New York (unregistered) in reply to AGlezB

    I knew a whole organization of developers who coded like this, particularly the aversion to variables. Seems they all took the same class where they learned that variables in general were bad. They use up bits, don't you know? So it was not uncommon to see the same property reference chain repeated a dozen times in a method.

  • (nodebb) in reply to Athanasius

    I'd hate to see what they did with getting day before. I have recently had to deal with a problem where some code was quite good at subtracting one day and if it was the first day of the month getting the last day of the previous month. It even worked for January. Except for one little omission. Resulting in our team receiving on 1st Jan a lot of information dated 365 days in the future...

    Even better, on this being pointed out, the response (after a few days) was "well, it's not happening today, so it's clearly been fixed"

  • (nodebb) in reply to Sou Eu

    I put the correct implementations already up there in my comment ;-)

  • Gnasher729 (unregistered)

    Letters for date conversions as defined in the Unicode standard include “y” and “Y” with slightly different meaning. One is the calendar year, changing on Jan 1st. The other is related to weeks. Jan 1st to Jan 3rd can be part of the last week of the previous year; Dec 29th to Dec 31st can be part of the first week of the next year. So if you use the wrong letter then you can get an unexpected year on up to three days around the year change.

    And every year a new generation of newbies falls in the trap and asks “why do I get the wrong year”. To make things worse, Europe and USA have different weeks, starting either Sunday or Monday. So someone in the USA can run into the problem and a developer in Europe may not be able to reproduce it.

  • MaxiTB (unregistered) in reply to Hanzito

    The interesting part is that c0 is a local variable, so the compiler cannot inline it.

    This is actually a pattern that is used in EF for LINQ expressions to distinguish between SQL parameters vs inlined literals. So if you put a const into a local variable instead of putting it directly into the expression, the expression parser creates an SQL parameter instead of hardcoding it into the SQL string. Which is obviously very important when it comes to SQL statement caches.

  • Hans (unregistered)

    The coder probably tried the format "yyyyMMddhhmmss" from the comment, noticed that it gave wrong values in the afternoon and thought: "stupid Microsoft cannot get their formats right, so I'll have to code everything myself"

  • Deeseearr (unregistered) in reply to Steve_The_Cynic

    It's C#, and the order of evaluation inside complex expressions is specified far more tightly than in C and C++.

    Specifications are a wonderful thing, but implementations can be something entirely different.

  • (nodebb) in reply to Deeseearr

    C# and the whole .net framework is an ISO specified standard. And as Steve said before, compared to other standards like C, it's super tight. It's also not a non-standardize proprietary language like Java; in fact everything is open source and you can look it up yourself ;-)

    When it comes to string concats, well, it's a simple left to right order and that's it. This is also default behavior for every other T operator+(T left, T right).

Leave a comment on “Timestamped File Name”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article