• (cs) in reply to Evan
    Evan:
    Jay:
    anonymous:
    I feel like the compiler should be smart enough to know that a dozen different copies of Now.DateTime that aren't used more than once and have no explicit delay between their invocation should really all be the same DateTime object.

    How would it know?

    If you write

    int hour=DateTime.Now.hour;
    int minute=DateTime.Now.minute;
    

    I'd guess you want the hour and minute for the same time.

    To play devil's advocate for a second you could at least argue that the compiler should merge DateTime.Now objects iff there is no intervening sequence point. (I don't know if C# has the notion of sequence points, but let's roll with this for a sec.) So foo(DateTime.Now.Hours, DateTime.Now.Minutes,...) has no sequence point between DateTime.Now invocations so they would share an object, but yours does have a sequence point between them so they would be different objects.

    I think there are a number of reasons this is a kind of dumb suggestions, but the above would at least be workable...

    Evan:
    I forgot to say: for my suggestion to work, you'd perhaps have to define DateTime.Now.xxxx properties so that they didn't inject sequence points for purposes of that behavior.

    I was thinking that may be a possibility and that the code may just be overly verbose, but the comment that I quoted above shows that this is not the case. And even if it was, the code would still count as a WTF, just not a potential source of issues.

  • (cs)

    Bonus WTF from the same app:

    string filename = string.Format("{0} Error Log - {1}-{2}-{3}.txt", applicationName, DateTime.Now.Month, DateTime.Now.Day, DateTime.Now.Year);

    Instead of something like:

    string filename = string.Format("{0} Error Log - {1:M-d-yyyy}.txt", applicationName, DateTime.Now);
  • EvilSnack (unregistered) in reply to Scourge of programmers.
    Scourge of programmers.:
    I believe the objective here is make the code unmanageable for the next programmer who steps in.
    That's management's job.
  • F (unregistered) in reply to Tom
    Tom:
    The last one is not that much of a WTF... None of the suggested existing types provide the same behavior. - Tuples don't override the equality and inequality operator, and since it's a class, these operators perform a reference equality - KeyValuePair doesn't override them either, and since it's a struct, there is no default implementation

    On the other hand, it is a mild WTF for these reasons:

    • The overriding of Equals and GetHashCode is redundant: for value types, the default implementation of these methods already take each field into account
    • public fields are bad; public mutable fields are very bad; public mutable fields in a struct should be a firing offense.

    I have to disagree on the last one.

    It should be a hanging offence.

  • (cs) in reply to TGV
    TGV:
    Tom:
    public mutable fields are very bad; public mutable fields in a struct should be a firing offense.
    Are you one of those people that make every member private and then add public getters and setters that do nothing but { return member; } and { member = value; }?

    Are you one of those people who declares every member public and accesses them directly without using getters or setters? Excuse me, I've just been sick in your mouth.

  • (cs) in reply to anonymous
    anonymous:
    I feel like the compiler should be smart enough to know that a dozen different copies of Now.DateTime that aren't used more than once and have no explicit delay between their invocation should really all be the same DateTime object.

    No, it has to look it up, not use the saved result.

    Have you never noted the time a procedure started and then at the end computed the elapsed time as Datetime.Now - the start time? Use the saved copy and you would always get zero.

  • Bill C. (unregistered) in reply to Evan
    Evan:
    TGV:
    Are you one of those people that make every member private and then add public getters and setters that do nothing but { return member; } and { member = value; }?
    How are you supposed to run an enterprise if you willy-nilly make everything public?
    Now I can't figure out if I was coming or going. Can we pleaase return to that hash discussion?
  • Amy (unregistered) in reply to Tom

    "public fields are bad; public mutable fields are very bad; public mutable fields in a struct should be a firing offense."

    You're clearly someone who has never had to write high performance C#. Classes have overhead. Properties have overhead. Readonly fields have overhead. Public mutable fields in a struct are common when performance is critical.

  • Greg (unregistered) in reply to F
    F:
    Tom:
    The last one is not that much of a WTF... None of the suggested existing types provide the same behavior. - Tuples don't override the equality and inequality operator, and since it's a class, these operators perform a reference equality - KeyValuePair doesn't override them either, and since it's a struct, there is no default implementation

    On the other hand, it is a mild WTF for these reasons:

    • The overriding of Equals and GetHashCode is redundant: for value types, the default implementation of these methods already take each field into account
    • public fields are bad; public mutable fields are very bad; public mutable fields in a struct should be a firing offense.

    I have to disagree on the last one.

    It should be a hanging offence.

    Unless you care about performance.

  • foxyshadis (unregistered) in reply to Matt Westwood
    Matt Westwood:
    TGV:
    Tom:
    public mutable fields are very bad; public mutable fields in a struct should be a firing offense.
    Are you one of those people that make every member private and then add public getters and setters that do nothing but { return member; } and { member = value; }?

    Are you one of those people who declares every member public and accesses them directly without using getters or setters? Excuse me, I've just been sick in your mouth.

    You and Tom and I appear to have entirely different ideas of what a struct should be. In my world:

    struct: A warehouse of data that needs no special manipulation, sanity checking, or side effects. It just is, and should be treated as much as possible like a useful relic of C.

    class: Everything else.

    Built-in initializer and cleanup functions are handy, but when all it does is hold a bunch of data that other things use, there's no point in not being public. Especially if you're making simple getter/setters equivalent to a public member, just to pretend to be pure.

    In any modern language you can transparently convert a public member to a property when you need sanity checks or side effects, anyway. (C++ really shows its age here.) If you are writing an external ABI I'd hope you'd figure out what you need to do ahead of time.

  • Walky_one (unregistered)

    public struct Rectangle { public int Left; public int Right; public int Top; public int Bottom;

    public int Width { [...] }

    [...] }

    Yes it has public fields. So what? What's the "additional worth" of properties here?

    AFAIK there are two differences:

    1. Properties ensure that I never can use the fields as out- or ref-parameters. Not sure I see that as advantage. though.
    2. Some technologies like WPF require properties for bindings (But IMO using stuff like WPF-Bindings is a much more serious offense. At least hanging and shooting at the same time...)
  • nobulate (unregistered) in reply to Jeremy
    Jeremy:
    Surely there's no need to implement both == AND !=....right?

    The Real WTF is that it could have simply been implemented like this:

           public static bool operator !=(StringString dataItem1, StringString dataItem2)
           {
               return (!dataItem1 == dataItem2);
           }
    
  • Walky_one (unregistered) in reply to nobulate
    nobulate:
    Jeremy:
    Surely there's no need to implement both == AND !=....right?

    The Real WTF is that it should have simply been implemented like this:

           public static bool operator !=(StringString dataItem1, StringString dataItem2)
           {
               return (!dataItem1 == dataItem2);
           }
    

    FTFY (It's always fun to debug strange errors because of slight discrepancies in equality/inequality checks or other compare operators)

  • Muphry (unregistered) in reply to Walky_one
    Walky_one:
    nobulate:
    Jeremy:
    Surely there's no need to implement both == AND !=....right?
    The Real WTF is that it should have simply been implemented like this:
           public static bool operator !=(StringString dataItem1, StringString dataItem2)
           {
               return (!dataItem1 == dataItem2);
           }
    FTFY (It's always fun to debug strange errors because of slight discrepancies in equality/inequality checks or other compare operators)
    No you didn't fix it (It's always fun to debug strange errors because of slight discrepancies in priorities of operators)
  • Walky_one (unregistered) in reply to Muphry

    I know: I posted my comment. Looked at it once more and found that one too. But was already too late (and too lazy to write a second one).

  • Jay (unregistered) in reply to Valued Service
    Valued Service:
    Jay:
    Dwedit:
    For a reference type, the rule of GetHashCode is that is must never change, otherwise you can't put it in a dictionary/hashset as the key. But I guess it's okay for a value type.

    Umm ... no. If the state of the object changes, I'd expect the hash code to change, or at least to possibly change.

    In this example, if the value of "key" is changed, I wouldn't want or expect the hash code to remain the same.

    If you're building a dictionary of these objects that is keyed by hash code, than it probably makes no sense for the has code NOR the key to change. It might or might not make sense for the value to change, depending on what you're doing. So you might say that once the object is created the key can never change, and that the hash code is derived only from the key. Or you might say neither key nor value can change. But I doubt I would allow the key to change but keep the hash code constant. That would not seem to make sense.

    If you're not building a dictionary and you can freely change the key and/or value of the object, then it would almost certainly not make sense for the hash code to be constant regardless of changes to the key and value.

    Maybe you could spin scenarios, depending on just what you're up to.

    You just walked in circles.

    If you imply a value is a key in code, and that it is used for the hash code, you shouldn't allow it or the hash code to change.

    That's just being a polite coder.

    The programmer should have just returned key in the hash.

    If they wanted to have both values in the hash, and intend the whole object to be a key, they should have just used the default object.GetHashCode();

    Either way, they've circumvented all the default logic that provably solves their problem.

    There are certainly times when you want to make an object immutable. There are times when you want to have a "key" value that is immutable while other elements of the object state are not. But it is not at all clear that all objects should always be immutable. Yes, if you're going to add them to a hash table, they'd better have some sort of key that is immutable. But if that's not how the object is used, why impose such a constraint on it? I've built lots of objects that aren't immutable, and where that's a very good thing because it would be a major pain to have to recreate the object every time something changes.

  • Gumpy Gus (unregistered)

    The same thing happens in the old IBM PC. The upper and lower halves of the time are kept in separate places, so if you read one the other has a chance of rolling over on you. Time going backwards does really bad things to task schedulers! I forget how we fixed this, it might have been as kludgy as seeing if the lower half was just before or after flipping over, we reread everything until we're sure we got a good reading.

  • Jay (unregistered) in reply to TGV
    TGV:
    Tom:
    public mutable fields are very bad; public mutable fields in a struct should be a firing offense.
    Are you one of those people that make every member private and then add public getters and setters that do nothing but { return member; } and { member = value; }?

    Of course! Because if you write

    public int foo;
    

    That's terrible, because outside objects can be changing the value of foo at any time.

    But

    private int foo;
    public void setFoo(int value) {foo=value;}
    public int getFoo() {return foo;}
    

    Whew, now that solved the problem. This is a completely different way of making classes interact.

    In Visual Basic they've added a nice little feature where you don't even have to write simple getters and setters. If you declare something to be a "property", the getters and setters are generated automatically. So in VB:

    public foo as integer;
    

    That would be horrible and evil and non-enterprisy. But

    public property foo as integer;
    

    Now all those problems go away, the grass turns green and the unicorns come out to frolic in the sunshine.

  • Jay (unregistered) in reply to Greg
    Greg:
    F:
    Tom:
    The last one is not that much of a WTF... None of the suggested existing types provide the same behavior. - Tuples don't override the equality and inequality operator, and since it's a class, these operators perform a reference equality - KeyValuePair doesn't override them either, and since it's a struct, there is no default implementation

    On the other hand, it is a mild WTF for these reasons:

    • The overriding of Equals and GetHashCode is redundant: for value types, the default implementation of these methods already take each field into account
    • public fields are bad; public mutable fields are very bad; public mutable fields in a struct should be a firing offense.

    I have to disagree on the last one.

    It should be a hanging offence.

    Unless you care about performance.

    Or about not cluttering up your code with getters and setters that give exactly the same results as a simple assignment. Or wasting your life writing such functions.

    Here we are in a thread ridiculing programmers who took five lines of code to do what they could have done in one, and someone posts why he thinks it's a great idea to use five lines of code to do what you could have done in one.

    Please explain to me what problem is created by making a variable public that is solved by making it private with a getter and setter. Not, "Because somebody wrote a rule in a book that we shouldn't make variables public and this lets me do what I need to do without breaking the rule." I mean, tell me what actually bug or maintenance problem or whatever is fixed by using get/set instead of a public variable.

    Now if you have a getter or setter that does validity checks or has side effects, that's a different story. I'm saying, compare "public int x" to "public void setX(int value) {x=value;} public int getX() {return x;}".

  • Jay (unregistered) in reply to Walky_one
    Walky_one:
    kilroo:
    Walky_one:
    And as a little side effect you can even get nice stuff like 31 of february if you time it right.

    You're getting the month before the day.

    You're right. Just found that C# strictly defines the order in which the parameters are resolved (I originally thought it would be compiler magic...)

    I was thinking that I wonder if order of evaluation of parameters is defined in the language spec. Of course just because C# does -- I assume you are correct in saying that -- doesn't necessarily mean that every language does.

    In any case, I would be very reluctant to write code like:

    int x=0;
    doSomething(++x,++x);
    

    Does that get called with 1 and 2 or 2 and 1? Or something else? Even if it is, in fact, strictly defined in the language spec, I think many programmers would not be sure of such a technicality, and it would be potentially confusing. It's like putting parentheses in complex expressions: I often put in parentheses that are not strictly necessary because the default evaluation order would be right anyway, but just as a help to future programmers who may not remember whether xor comes before or after multiply and that sort of thing. (Hey, I just got burned on this when I wrote "index and 1 = parity". I was thinking "(index and 1) = parity" but the compiler interpreted it as "index and (1 = parity)".)

  • (cs) in reply to Jay
    Jay:
    In any case, I would be very reluctant to write code like:
    int x=0;
    doSomething(++x,++x);
    

    Does that get called with 1 and 2 or 2 and 1? Or something else?

    I haven't dug far enough into C#'s semantics (etc.) to say for sure, but I can tell you that in C++ this exhibits undefined behaviour even before you reach the inside of doSomething(). There's a sequence point at the ; at the end of the definition of x, and another just before the call to doSomething, and x is modified in two places in between, and more to the point, it is read (for the second modification) in a way that is unrelated to making the first modification. Even if I was wrong, and there is an additional sequence point at the comma, the order of evaluation of the arguments is left up to the compiler, and doing anything that might assume anything about the values of the parameters once inside doSomething() will invoke undefined behaviour.

    Don't run this code (nor the rest of any program that contains it) on a DeathStation 9000, unless you want the crispy-fried blood of millions on your hands.

  • (cs) in reply to Evan
    Evan:
    To play devil's advocate for a second you could at least argue that the compiler should merge DateTime.Now objects iff there is no intervening sequence point.

    What do you mean by sequence point? In C, each of those semicolons would be a sequence point.

    However you phrase it, it's not an optimization so much as it's a patch for buggy programs. It's very much a special case; I can't imagine another volatile function where doing that wouldn't be a horrible bug in the compiler.

  • Walky_one (unregistered) in reply to Jay
    Jay:
    Walky_one:
    You're right. Just found that C# strictly defines the order in which the parameters are resolved (I originally thought it would be compiler magic...)

    I was thinking that I wonder if order of evaluation of parameters is defined in the language spec. Of course just because C# does -- I assume you are correct in saying that -- doesn't necessarily mean that every language does.

    C++ -> The compiler is free to reorder the calls as necessary.

    And no: Writing code like "i++ * i * ++i" is never a good idea. No matter if the order is defined or not.

  • (cs) in reply to abarker
    abarker:
    Whoops, missed those. I haven't had a chance to use LINQ in a while due to the standards where I work, but I use plenty of Lambda expressions for cross-threading. Guess my rustiness in that area is showing this morning.
    What kind of place has coding standards that allow Lambdas but don't allow LINQ statements? Do they really prefer you write loops through collections rather than using a simple Where/Lambda combination?
  • Paul Neumann (unregistered) in reply to Steve The Cynic
    Steve The Cynic:
    Jay:
    In any case, I would be very reluctant to write code like:
    int x=0;
    doSomething(++x,++x);
    

    Does that get called with 1 and 2 or 2 and 1? Or something else?

    I haven't dug far enough into C#'s semantics (etc.) to say for sure, but I can tell you that in C++ this exhibits undefined behaviour even before you reach the inside of doSomething(). There's a sequence point at the ; at the end of the definition of x, and another just before the call to doSomething, and x is modified in two places in between, and more to the point, it is read (for the second modification) in a way that is unrelated to making the first modification. Even if I was wrong, and there is an additional sequence point at the comma, the order of evaluation of the arguments is left up to the compiler, and doing anything that might assume anything about the values of the parameters once inside doSomething() will invoke undefined behaviour.

    Don't run this code (nor the rest of any program that contains it) on a DeathStation 9000, unless you want the crispy-fried blood of millions on your hands.

    program:
    void Main()
    {
    int x = 0;
    x.Dump("pre");
    doSomething("postfix", x++, x++);
    doSomething("prefix", ++x, ++x);
    x.Dump("post");
    // ++x.Dump(); //Compiler Error: The operand of an increment or decrement operator must be a variable, property or indexer
    (++x).Dump("inline");
    }

    void doSomething(string desc, int a, int b) { a.Dump(desc + ": a"); b.Dump(desc + ": b"); }

    results:
    pre
    0

    postfix: a 0

    postfix: b 1

    prefix: a 3

    prefix: b 4

    post 4

    inline 5

    I find that the . operator has higher precedence than a prefix operator on line 8 to be the most interesting result. The postfix operator had higher precedence than the . operator (not demonstrated here).

  • Norman Diamond (unregistered) in reply to Gumpy Gus
    Gumpy Gus:
    The same thing happens in the old IBM PC. The upper and lower halves of the time are kept in separate places, so if you read one the other has a chance of rolling over on you. Time going backwards does really bad things to task schedulers! I forget how we fixed this, it might have been as kludgy as seeing if the lower half was just before or after flipping over, we reread everything until we're sure we got a good reading.
    Read the upper half. Read the lower half. Read the upper half again. If the upper half is the same as before, you're done. Otherwise read the lower half again. Unless your computer is inordinately slow, you're done.
  • Norman Diamond (unregistered) in reply to Valued Service
    Valued Service:
    That's just being a polite coder.
    Oxymoron alert.
  • Norman Diamond (unregistered)

    I can't figure out what language Paul Newman's code is. If it's based on C or C++, the output is a perfectly fine possibility because undefined behaviour can have any result. Paul Newman's boss will be happy because the program will produce desired results in QA, but customers will be unhappy because the program will format their hard drives.

  • Alissa (unregistered) in reply to anonymous
    void Log(String s) {
      Console.WriteLine(s);
      Thread.Sleep(1000);
    }
    
  • anonymous (unregistered) in reply to Alissa
    Alissa:
    void Log(String s) {
      Console.WriteLine(s);
      Thread.Sleep(1000);
    }
    
    What's your point, Alissa?
  • (cs) in reply to anonymous
    anonymous:
    Alissa:
    void Log(String s) {
      Console.WriteLine(s);
      Thread.Sleep(1000);
    }
    
    What's your point, Alissa?

    She'll tell you in a second

  • Haxy (unregistered) in reply to Jay
    Jay:
    Dwedit:
    For a reference type, the rule of GetHashCode is that is must never change, otherwise you can't put it in a dictionary/hashset as the key. But I guess it's okay for a value type.

    Umm ... no. If the state of the object changes, I'd expect the hash code to change, or at least to possibly change.

    Partially correct. HashCode and Equals is an entire can of worms. If an object's equality does not change, the hashCode should also not change. However, two objects with the same hashCode may not also be equal, as was previously pointed out.

    Work with hibernate on a large scale and you'll encounter this a bit. Associations ( Colony hasMany Ants ) are stored in hashTables. If you add a new Ant to a Colony you can end up overwriting (deleting and replacing) an existing Ant, or duplicating ants if you were to re-add an existing one if you don't setup hashCode correctly. That's why you need to be careful about updating hashCodes for every little object change.

    The best approach is to setup hashCode and equals to match business logic equals, not programmer logic equals. The identity, for example, should be excluded from hashCode and only be used in equals if both objects have it set.

    http://t.co/GLocCCg0k0

    @EqualsAndHashCode in Groovy is your friend :-)

  • Haxy (unregistered) in reply to Haxy

    I should add that hashCodes are used to generate ETags for REST APIs in some projects as well. One thing I like about this approach is it helps the programmer really think about what "change" is in an object.

  • anonymous (unregistered) in reply to chubertdev
    chubertdev:
    anonymous:
    Alissa:
    void Log(String s) {
      Console.WriteLine(s);
      Thread.Sleep(1000);
    }
    
    What's your point, Alissa?

    She'll tell you in a second

    Is that a male's second or a female's second?

  • Anonymous Coder (unregistered) in reply to Jay
    Jay:
    Or about not cluttering up your code with getters and setters that give exactly the same results as a simple assignment. Or wasting your life writing such functions.

    Here we are in a thread ridiculing programmers who took five lines of code to do what they could have done in one, and someone posts why he thinks it's a great idea to use five lines of code to do what you could have done in one.

    Please explain to me what problem is created by making a variable public that is solved by making it private with a getter and setter. Not, "Because somebody wrote a rule in a book that we shouldn't make variables public and this lets me do what I need to do without breaking the rule." I mean, tell me what actually bug or maintenance problem or whatever is fixed by using get/set instead of a public variable.

    Now if you have a getter or setter that does validity checks or has side effects, that's a different story. I'm saying, compare "public int x" to "public void setX(int value) {x=value;} public int getX() {return x;}".

    I got to agree with you on that. Plus: You successfully abstracted away the inards of your class, why do you want to go on and expose them with loads of getters and setters?

  • qbolec (unregistered) in reply to Norman Diamond
    Norman Diamond:
    Gumpy Gus:
    The same thing happens in the old IBM PC. The upper and lower halves of the time are kept in separate places, so if you read one the other has a chance of rolling over on you. Time going backwards does really bad things to task schedulers! I forget how we fixed this, it might have been as kludgy as seeing if the lower half was just before or after flipping over, we reread everything until we're sure we got a good reading.
    Read the upper half. Read the lower half. Read the upper half again. If the upper half is the same as before, you're done. Otherwise read the lower half again. Unless your computer is inordinately slow, you're done.
    hi = getHi();
    loop{
      lo = getLo();
      newHi = getHi();
      if(newHi==hi){
        return combine(lo,hi)
      }
      hi=newHi;
    }
    
  • anonymous (unregistered) in reply to qbolec
    qbolec:
    Norman Diamond:
    Gumpy Gus:
    The same thing happens in the old IBM PC. The upper and lower halves of the time are kept in separate places, so if you read one the other has a chance of rolling over on you. Time going backwards does really bad things to task schedulers! I forget how we fixed this, it might have been as kludgy as seeing if the lower half was just before or after flipping over, we reread everything until we're sure we got a good reading.
    Read the upper half. Read the lower half. Read the upper half again. If the upper half is the same as before, you're done. Otherwise read the lower half again. Unless your computer is inordinately slow, you're done.
    hi = getHi();
    loop{
      lo = getLo();
      newHi = getHi();
      if(newHi==hi){
        return combine(lo,hi)
      }
      hi=newHi;
    }
    
    How many times do you expect getHi() to change if you're polling it in a loop? That would fall under the "unless your computer is inordinately slow" part.

    Norman's code assumes that the clock can only tick at most once in the time it takes to execute your function:

    oldHi = getHi();
    lo = getLo();
    hi = getHi();
    if (hi != oldHi) {
      lo = getLo();
    }
    return combine(lo, hi);
    
  • rwm (unregistered) in reply to Jay
    Jay:
    Now if you have a getter or setter that does validity checks or has side effects, that's a different story. I'm saying, compare "public int x" to "public void setX(int value) {x=value;} public int getX() {return x;}".

    What happens when you realize you need to add validity checks and/or side effects, and you have thousands of direct references to the field?

    C# handles this quite well by generating the private backing field and the default getters & setters, so you don't have to clutter your code or waste time writing boilerplate.

  • foodbar (unregistered) in reply to lorna
    lorna:
    Hashes should be xored, not added. Adding up evenly distributed values doesn't result in an even distribution.
    xor is its own inverse; thus, a xor a is 0. So if the hash for a two tuple {a,b} is hash(a) xor hash(b), then it will be 0 for every a==b. Not good.

    If you add instead, the hash for {a,b} will be 2*hash(a) for every a==b. That's a bit more reasonable.

  • anonymous (unregistered) in reply to foodbar
    foodbar:
    lorna:
    Hashes should be xored, not added. Adding up evenly distributed values doesn't result in an even distribution.
    xor is its own inverse; thus, a xor a is 0. So if the hash for a two tuple {a,b} is hash(a) xor hash(b), then it will be 0 for every a==b. Not good.

    If you add instead, the hash for {a,b} will be 2*hash(a) for every a==b. That's a bit more reasonable.

    Instead of this:

    hash(a) xor hash(b)

    Do this:

    hash(concatenate(salt[0], a)) xor hash(concatenate(salt[1], b))

    It is highly unlikely to be 0.

Leave a comment on “The Long Way”

Log In or post as a guest

Replying to comment #:

« Return to Article