What’s Wrong With This Picture?

12 commentsWritten on April 27th, 2010 by
Categories: What's Wrong With This Picture

crap 

Or maybe the question should be: what’s right about this?

« Why I’ve Been So Quiet LatelyLooking For Agatha Testimonials »

This entry was posted on Tuesday, April 27th, 2010 at 08:47 and is filed under What's Wrong With This Picture. You can subscribe to any comments to this entry via RSS 2.0. You can leave a response, or trackback from your own site.

  • Kenneth

    DirectoryInfo is a derived class from FileSystemInfo, so you don’t actually need the “if” statement.

  • tcmaster

    this method is not virtual, so this will always points to the exact class. in other words, the if statement will always go into 1 branch.

  • Koen

    DirectoryInfo is sealed, so “this is DirectoryInfo” == always false (unless this code is from the DirectoryInfo class)

  • leo

    We should never do 2 different things on a simple setter.

  • Magesh

    “Directory.SetLastWriteTimeUtc()” can be used for changing the create time attribute of both directories and files and hence the “File.SetLast…” is unnecessary.

  • Adam

    It uses the is operator on itself in an if. Instead, the method should be overridden, does the same thing.

  • Dalibor

    You should call DateTime.ToUniversalTime before setting time, but if you are already doing this before calling the method then it would be fine.

  • Dalibor

    I meant

    value = value.ToUniversalTime();

  • http://odalet.wordpress.com Olivier

    The problem is that it works!
    IMHO, it should have been coded using virtual/override, not by guessing about the real type of the object… this is simply ugly.

    Btw, the code is the reflectorized setter (I recognized the yellow background and the set_) of System.FileSystemInfo.LastWriteTimeUtc property.

    Another point: the -1 value is used to say “Yes I’m initialized”… weird (and setting any of the time properties of FileSystemInfo sets the _dataInitialised member to true (-1)).

  • http://weblogs.asp.net/bleroy Bertrand Le Roy

    There is a typo in “dataInitialised”.

  • http://benpittoors.wordpress.com den Ben

    I can’t think of anything that is ‘right’ about it… no matter how hard I try.

    Oh yes! Got one: It’s properly indented! :)

  • http://davybrion.com Davy Brion

    IMO, there is indeed absolutely nothing right about this, and every single line is pretty much horrible, though not exactly for the reasons that have been pointed out in the comments

    The code is indeed the setter of the LastWriteTimeUtc property of the FileSystemInfo class. DirectoryInfo inherits from FileSystemInfo, so the fact that the FileSystemInfo class even _knows_ about it is just plain wrong. It would’ve been much cleaner if it were a virtual method which DirectoryInfo would override.

    But, it does actually work since DirectoryInfo does not define a LastWriteTimeUtc property itself and it inherits it from FileSystemInfo. So the if statement actually works, though it is completely unnecessary as Magesh pointed out since Directory.SetLastWriteTimeUtc works for both files and folders.

    The fact that the real logic is hidden in static methods is something i’m not fond of either, though i guess that’s not to unusual with Microsoft code ;)

    and setting dataInitialised to -1 is just horrible

Meta

Recent Posts

Categories