• (cs) in reply to Miriam
    Miriam:
    I don't understand at all why so many people do such things. Even Eclipse with its sort of overloaded GUI manages to give you a good overview about things inside a class. You really shouldn't add those comments.

    Some of my coworkers love putting #Regions for exactly such purposes in their C♯ and VB.NET. It's horrible!

    If the biggest WTF in your code is #regions, you've got some pretty good code.

  • Turlingdrome23 (unregistered) in reply to foo AKA fooo
    foo AKA fooo:
        final int prime = 42;
        int result = 1;
        result = prime * result + ...
    
    That's really brillant!

    First, as everyone knows 42 is not a prime because it's 6 * 9.

    However, a prime is a number that's divisible by itself and 1. Therefore, multiplying itself by 1 can turn any number into a prime. That's a really clever and little known trick. The true master shines here!

    Wait a minute.... a prime is a number that's divisible by itself and one? If it's [evenly] divisible by itself, then it MUST equal itself. So, for any X, where X=X, then x is prime! No need to multiply by one, all numbers are prime.

    captcha: causa - Causa I said so.

  • anonymous (unregistered) in reply to Turlingdrome23
    Turlingdrome23:
    foo AKA fooo:
        final int prime = 42;
        int result = 1;
        result = prime * result + ...
    
    That's really brillant!

    First, as everyone knows 42 is not a prime because it's 6 * 9.

    However, a prime is a number that's divisible by itself and 1. Therefore, multiplying itself by 1 can turn any number into a prime. That's a really clever and little known trick. The true master shines here!

    Wait a minute.... a prime is a number that's divisible by itself and one? If it's [evenly] divisible by itself, then it MUST equal itself. So, for any X, where X=X, then x is prime! No need to multiply by one, all numbers are prime.

    captcha: causa - Causa I said so.

    You're spreading an awful little bit of humour on an awful big bagel there, bud.

  • Hasse de great (unregistered) in reply to Turlingdrome23
    Turlingdrome23:
    foo AKA fooo:
        final int prime = 42;
        int result = 1;
        result = prime * result + ...
    
    That's really brillant!

    First, as everyone knows 42 is not a prime because it's 6 * 9.

    However, a prime is a number that's divisible by itself and 1. Therefore, multiplying itself by 1 can turn any number into a prime. That's a really clever and little known trick. The true master shines here!

    Wait a minute.... a prime is a number that's divisible by itself and one? If it's [evenly] divisible by itself, then it MUST equal itself. So, for any X, where X=X, then x is prime! No need to multiply by one, all numbers are prime.

    captcha: causa - Causa I said so.

    ... and ONLY divisible by itself or one ...

  • fjf (unregistered) in reply to Fritz'dUpp
    Fritz'dUpp:
    In my universe, 6 * 9 = 54
    Don't worry. Your universe will soon be replaced by something even more bizarre and inexplicable.
  • Not Douglas Adams (unregistered) in reply to fjf
    fjf:
    Fritz'dUpp:
    In my universe, 6 * 9 = 54
    Don't worry. Your universe will soon be replaced by something even more bizarre and inexplicable.

    Hasn't that happened already?

  • Robert (unregistered)
    if (obj instanceof ZipCodeInfo) { return false; }

    This must be an optimization to avoid the expensive equals check.

  • Turlingdrome23 (unregistered) in reply to anonymous
    anonymous:
    Turlingdrome23:
    foo AKA fooo:
        final int prime = 42;
        int result = 1;
        result = prime * result + ...
    
    That's really brillant!

    First, as everyone knows 42 is not a prime because it's 6 * 9.

    However, a prime is a number that's divisible by itself and 1. Therefore, multiplying itself by 1 can turn any number into a prime. That's a really clever and little known trick. The true master shines here!

    Wait a minute.... a prime is a number that's divisible by itself and one? If it's [evenly] divisible by itself, then it MUST equal itself. So, for any X, where X=X, then x is prime! No need to multiply by one, all numbers are prime.

    captcha: causa - Causa I said so.

    You're spreading an awful little bit of humour on an awful big bagel there, bud.

    Were you objecting to my comment or Foo AKA Fooo? Just trying to simplify his math. Are you Canadian? That could explain it. You use "bud" and "bagel" but use the commonwealth "Humour".

  • Sarc (unregistered) in reply to C-Derb
    C-Derb:
    public boolean equals(Object obj) {
        if (this == obj) { 
          return true;
        }
        if (obj == null) {
          return false;
        }
        if (obj instanceof ZipCodeInfo) {
          return false;
        }
    
    return Equal.equals(this.getZipCode(), ((ZipCodeInfo) obj).getZipCode());
    

    }

    Bonus WTF points for comparing the two objects BEFORE checking if obj is null.
    because if "this" is null then we save ourselves a comparison

  • Johnny (unregistered) in reply to Miriam
    Miriam:
    I don't understand at all why so many people do such things. Even Eclipse with its sort of overloaded GUI manages to give you a good overview about things inside a class. You really shouldn't add those comments.

    Some of my coworkers love putting #Regions for exactly such purposes in their C♯ and VB.NET. It's horrible!

    Helps fold code in some IDE's and ignore bits that aren't interesting in others. Mind you, good consistent indenting (no mixing editors that means somethings are tab indented, some are space indented and some are all over the shop).

    having had to work in support in an environment where people frequently move files from the Unix build environment to Windoze just because they loke notepad for editing (ok, I'm jokling a little - I think they use VS for its syntac highlightind (which is perhaps even more laughable), I appreciate any attempt to make code easier to follow - even if it does bloat the source

  • Shawn Sean (unregistered)

    Perhaps in future we need references and explanation every time anyone refers to something that not everyone in the world has read.

    Apparently not anyone understood the reference to Hitchikers Guide - although I'll admit I've never read it and still picked the reference....but I'm just awesome!

  • foo AKA fooo (unregistered) in reply to Shawn Sean
    Shawn Sean:
    Perhaps in future we need references and explanation every time anyone refers to something that not everyone in the world has read.
    Amen.

    Reference: The Bible

  • (cs) in reply to foo AKA fooo
    foo AKA fooo:
    Shawn Sean:
    Perhaps in future we need references and explanation every time anyone refers to something that not everyone in the world has read.
    Amen.

    Reference: The Bible

    +1!!

  • lolatu (unregistered) in reply to foo AKA fooo
    foo AKA fooo:
    Shawn Sean:
    Perhaps in future we need references and explanation every time anyone refers to something that not everyone in the world has read.
    Amen.

    Reference: The Bible

    Zing! you win 1 whole internets for today.

  • Vogon (unregistered) in reply to sysKin
    sysKin:
    In case someone misses it: storing the same thing as strong reference (key to the map) and weak reference completely bypasses the point of weak references -- the object cannot be GCed.
    The whole program is garbage, just waiting to be replaced by something even more bizarre and inexplicable. Like this:
    human:
    final int prime = 42;
    Beware of human poetry, my fellow Vogons. It's the biggest factor in crashes we've been observing in recent cycles. Even the worst decomposition of Bach's dead body isn't as bad as their poetry.
  • The Crunger (unregistered)

    I really want to understand the full depth of this crime against Java.

    can't wait for java 7
    Is this a Remy-style snark that somehow escaped into the article, or is this valid syntax? (Perhaps like importing from the future in Python.)

    Also, is something deliciously bad going to happen to this code once they start using the only secure of java that exists? Correction -- I should say "most secure from the set of versions formally recognized by Oracle".

    class ZipCodeInfo implements ImmutableZipCodeInfo
    This seems to mean that ZipCodeInfo is a sub-class of ImmortalZipCode. Can you really subclass something to remove its inherent property of immutability? I would have thought the other way around.

    Also, and perhaps this is the point of the article's name, is nothing really jumps out about either of these things as being immutable.

    Thankfully, the ZipCodeInfo implemented equals() and hashCode() as it should
    Well, it implements these methods, which is perhaps enough to make it possible to create a HashMap, but given the WTF-ery in the equals method, how can this code even be close to working?

    I guess I'm interested in what bit of accidental tomfoolery causes this code to somehow work as hoped even 60% of the time (i.e. the functional level necessary to obtain approvals to offshore the code and make it Someone Else's Problem).

  • Vogon (unregistered) in reply to The Crunger
    The Crunger:
    This seems to mean that ZipCodeInfo is a sub-class of ImmortalZipCode.
    Close. The handcrafted code in 42.zip is immoral not immortal.
  • faoileag (unregistered) in reply to The Crunger
    The Crunger:
    class ZipCodeInfo implements ImmutableZipCodeInfo
    This seems to mean that ZipCodeInfo is a sub-class of ImmortalZipCode.
    It might seem so to you; in the Java world it means that class ZipCodeInfo is implementing the ImmutableZipCodeInfo interface.

    The naming convention would normally be something like:

    class ImmutableZipCodeInfo implements IImmutableZipCodeInfo
    but perhaps the developer didn't like the double letter at the beginning of the interface name.
  • anonymous (unregistered) in reply to Turlingdrome23
    Turlingdrome23:
    Were you objecting to my comment or Foo AKA Fooo? Just trying to simplify his math.
    Yours.
    Turlingdrome23:
    Are you Canadian? That could explain it. You use "bud" and "bagel" but use the commonwealth "Humour".
    Just trying to keep you guessing.
  • Your Name (unregistered) in reply to Anonymous
    Anonymous:
    Your Name:
    Coward:
    List l = new ArrayList();

    if(l != null)

    I was going to say that maybe he's hung up on a C++-ism, but that's not right there either; if new fails it throws a std::bad_alloc exception. Certainly you check for null there in C with malloc, I suppose.

    Well, you can overload the new operator to return NULL, yay!

    Well, yeah, C++ lets you do all kinds of dumb shit with operator overloading. You can even overload the bit shift operator to send strings to a stream, though I don't know why anybody would think THAT was a good idea.

  • Evan (unregistered) in reply to The Crunger
    The Crunger:
    This seems to mean that ZipCodeInfo is a sub-class of ImmortalZipCode. Can you really subclass something to remove its inherent property of immutability? I would have thought the other way around.
    IMO it's definitely not the other way around; it's a lot closer to being correct than backwards.

    Why? Suppose you had ImmutableThing as a subclass of Thing. Thing will have methods like setFoo. The subclass relationship says that ImmutableThing needs to obey the same contract as Thing, e.g. "calling setFoo(new_foo) will update foo." But ImmutableThing can't implement that interface, because then it wouldn't be immutable. So from a strict OO design perspective, ImmutableThing cannot be a subclass of Thing.

    Now, some times for one reason or another it's more expedient to ignore this problem and just have ImmutableThing implement setFoo as throw new UnsupportedOperationException(). This isn't necessarily a WTF, but at the same time pretty much any time you see something like UnsupportedOperationException that's an indication that you don't really have a subclass relationship.

    Well, what about the other way? Is Thing a subclass of ImmutableThing? Well, yes and no. From a 'local' perspective, it is. ImmutableThing will have accessors like getFoo(). But a (mutable) Thing can implement those functions just fine. So from that perspective, the subclass relationship in this direction is correct! It becomes incorrect if you look across calls, to properties such as "getFoo returns the same object on different calls". That will hold for ImmutableThing (because how would it change?) but not for Thing (because someone could setFoo in the interim). Is that property part of the interface? Depends how you look at it. I'd probably argue "yes" and that the Thing implements ImmutableThing relationship is technically not valid, but I'd also say the other position could be reasonably argued (especially in a language like Java where you don't have some super-ultra-fancy type system).

  • Evan (unregistered) in reply to Your Name
    Your Name:
    Well, yeah, C++ lets you do all kinds of dumb shit with operator overloading. You can even overload the bit shift operator to send strings to a stream, though I don't know why anybody would think THAT was a good idea.
    Because in a language that supports operator overloading (for other, very legit reasons) but before you have templates, overloading an operator is by far the best way to achieve extensible output. (In other words, you can define a means for your own classes to perform input/output.) Arguably that is even true after you have templates but before variadic templates, which of course were only added in C++11.

    IMO, that one benefit makes up for and then far surpasses the drawbacks in terms of compactness of iostreams-style IO vs printf-style IO.

  • Russ (unregistered)

    Think the original coder was trying to give a nod to the hitchhikers guide to the universe with that magic number of 42? Can't explain the *1 part but perhaps the 42 was written 'for the lolz'

  • Your Name (unregistered) in reply to Evan
    Evan:
    Your Name:
    Well, yeah, C++ lets you do all kinds of dumb shit with operator overloading. You can even overload the bit shift operator to send strings to a stream, though I don't know why anybody would think THAT was a good idea.
    Because in a language that supports operator overloading (for other, very legit reasons) but before you have templates, overloading an operator is by far the best way to achieve extensible output. (In other words, you can define a means for your own classes to perform input/output.) Arguably that is even true after you have templates but before variadic templates, which of course were only added in C++11.

    IMO, that one benefit makes up for and then far surpasses the drawbacks in terms of compactness of iostreams-style IO vs printf-style IO.

    Well, you have some problems with that.

    The first is that operator precedence doesn't change. The compiler will still treat the bitshift operator as a bitshift operator, regardless of the abominations that you're forcing it to perform. I need to toss in a quick debug statement to see if "status" is less than zero. No problem:

    std::cerr << status < 0;

    Oops.

    And, since cout << "Hello World" is the first line of C++ a new coder sees, they're encouraged to do stupid operator overloads from day one by the standard library itself. From there it's a matter of time before you see them try and overload the bitwise xor operator to do exponentiation. Then they get introduced to the "friend" keyword to implement that extensible stream feature on their new class.

    But hey, we have extensible type-checked IO, so forget these small problems, this is for production use! Except, of course, that iostreams-style IO is impossible to localize, so the entire facility is worthless outside of trivial example code using strings literal. Whoops! Guess somebody has to go back and change all of those calls to sprintf after all, so they can reference the correct language file.

  • Paul M (unregistered)

    The cache code is flat wrong.

    Most obviously, the ref will never get cleared because there is a hard reference to the zipcode object - it is being used as a key.

    To fix this, you use a WeakHashMap. But there's an error there too - there's nothing to stop the ref being cleared between the call to containsKey() and the call to get().

    What you have to do is get() the ref, if it is not null then get() the ZipInfo, and if the result of that is not null then return it. Otherwise, re-fetch.

    And it's still not threadsafe, if that's a thing. Which it probably is.

  • Paul M (unregistered) in reply to Hasse de great
    ... and ONLY divisible by itself or one ...
    Or to put it another way - it's only prime factor is itself. 1 is not a prime number and it's set of prime factors is the empty set.
  • Paul M (unregistered) in reply to Paul M
    To fix this, you use a WeakHashMap. But there's an error there too - there's nothing to stop the ref being cleared between the call to containsKey() and the call to get().
    I'll bet that's what the crack about java 7 is about. When he used a WeakHashMap, his code didn't work, and he thought it was because the implemetation in java 6 was buggy.
  • Paul M (unregistered)

    Oh, other problem is that weak references are aggressively cleared by the gc. This means that his "cached" values will be discarded as soon as his code has finished with them (unless there's threading going on). He should be holding soft references as the values in his weak hash map.

  • Cube (unregistered) in reply to Fredrik
    Fredrik:
    Squiggle:
    Daniel Bos:
    zipCache.put(newZipInfo, new WeakReference(newZipInfo));
    

    That's a beauty!

    Wait, what?

    WeakReferences allow garbage collection of an object, so it's commonly used in stuff like caches (we don't really need a reference to it, but hey, if you haven't collected it already, we'll reuse it instead of reloading it). Only problem is, the key is the same object being referenced, so it'll never ever be eligible for collection. This snipped is just plain dumb.

    Almost; he's only adding the zipcode as a key if it was not found before (by hash/equals). So in fact, he only adds it the first time it's encountered.

    In fact he built his own interning system for zipcodes. I've seen it before.

    The real issue is he should've used weakhashmap as well, to have weak references to the keys.

    And of course the shared instance makes for a lot of threading fun :)

  • Evan (unregistered) in reply to Your Name
    Your Name:
    The first is that operator precedence doesn't change. The compiler will still treat the bitshift operator as a bitshift operator, regardless of the abominations that you're forcing it to perform. I need to toss in a quick debug statement to see if "status" is less than zero. No problem:

    std::cerr << status < 0;

    Oops.

    I never said that iostreams was perfect, just that I think it's clearly better than any printf-like function you could write in earlier C++ (when iostream was created). Let's think about what the alternatives are for printing your own classes. In early, ARM C++ (by which iostreams in an earlier incarnation were already present):

    1. Use printf for printing non-primitive types, then make a different function call for printing your type, then go back to printf. ('printf(...); print_my_object(o); printf(...);')

    2. Use printf, use a %s operator, and have your class return a string, then convert that string to a C string. ('printf("%s", myobj.to_string().c_str());')

    3. Similar to 2, but have your class maintain a C string representation that it can return directly. ('printf("%s", myobj.c_str());')

    4. Overload a binary function 'f(str, obj)'. This is like the expansion of the overloaded << and >> operators. ('output(output(cout, obj1), obj2);')

    The addition of templates added a new option:

    1. Make a function 'template{typename T1=NullType, typename T2=NullType, ..., typename TN=NullType} printf(const char* fmt, T1 o1=NullType(), T2 o2=NullType(), ..., TN on=NullType())' for some fixed N. The implementation would internally expand to something like #4 above. (Curly braces for the template because TRTWF is this forum software. Also, this may be slightly off; I forget exactly how variadic templates are emulated in C++98, but that's something like the usual way.)

    The addition of variadic templates added a new option:

    1. Do the same as 5 with variadic templates and, ideally, perfect forwarding.

    My evaluation is that the syntactic obnoxiousness of #1 and #4 are way worse than the precedence issues and verbosity with ostream. #3 is actually decent all around if you can do it, but there are issues including potential race conditions (if the object has to update its string) and the fact that most of the time there's no reason to keep that string representation around. #2 is the most general and only mildly annoying syntactically, but it requires building the entire object's string representation in memory before outputting, which can occasionally be an issue. It also begs the question of how you construct that string in the first place... the best way of which in the standard library (at least pre-C++11, not sure if there's something now) is stringstream! (Though a version of sprintf that returns a std::string would be easily possible to write even in early C++.) Still, I'd take iostreams over #2 most of the time, and would definitely take it if compilers didn't have format-string-mismatch warnings (which I think was probably common at the time, considering that one of the other big benefits touted for iostreams was type safety). Maybe my statement that the << >> operators was 'by far' the best of the mechanisms available in ARM C++ was a bit strong.

    It's only where you get to #5, where you can write a print function that you can call like, say, the Python format function -- 'format("{} + {} = {}", a, b, a + b)', including if 'a' and 'b' are of class type -- where you get to something that I think presents a really strong case for being better than iostreams. And even it has the limitation that it has some edge-case restrictions, most obviously the maximum number of output parameters, which must be fixed.

    Then they get introduced to the "friend" keyword to implement that extensible stream feature on their new class.
    "friend" is not a bad feature, and using it for the stream functions is, IMO, almost 100% fine. Or maybe you don't like it. Then just add a public print(ostream &) function to your class and forward to it.
    Except, of course, that iostreams-style IO is impossible to localize, so the entire facility is worthless outside of trivial example code using strings literal.
    Do you really work in some crazy world where everything needs to be localized? Even if you think that every program should be internationalized from the start because you never know when you'll need it (perhaps a dubious claim, but I'll allow it), IMO there is still plenty of output that has little, no, or negative need to be localized. Log messages meant for the developer don't need to be localized. I/O to and from data files that aren't even meant much for human consumption in the first place definitely don't need to be localized.
  • (cs) in reply to Paul M
    Paul M:
    I'll bet that's what the crack about java 7 is about. When he used a WeakHashMap, his code didn't work, and he thought it was because the implemetation in java 6 was buggy.
    Nope, that quip was about Java 7's diamond operator, which would have allowed to write that line as:
    private Map‹ImmutableZipCodeInfo, WeakReference‹ImmutableZipCodeInfo›› zipCache = new HashMap‹›();
    
    while in Java 5 and 6 it would be the following monstrosity:
    private Map‹ImmutableZipCodeInfo, WeakReference‹ImmutableZipCodeInfo›› zipCache =
       new HashMap‹ImmutableZipCodeInfo, WeakReference‹ImmutableZipCodeInfo››();
    
  • (cs) in reply to faoileag
    faoileag:
    The naming convention would normally be something like:
    class ImmutableZipCodeInfo implements IImmutableZipCodeInfo
    but perhaps the developer didn't like the double letter at the beginning of the interface name.

    You normally see "I" at the beginning of interface names in Java? I don't recall ever seeing that. Certainly the Java API itself doesn't do that. Do you preface class names with "C"?

    Does anyone else find the idea of an interface defining something as immutable to be stupid?

  • Neil (unregistered) in reply to faoileag
    faoileag:
    if (obj instanceof ZipCodeInfo) {
      return false;
    }
    Shouldn't that be better "if (!obj instanceof ZipCodeInfo) {"
    !obj is always instanceof Boolean, not ZipCodeInfo. Did you mean !(obj instanceof ZipCodeInfo)?
  • (cs) in reply to Neil
    Neil:
    faoileag:
    if (obj instanceof ZipCodeInfo) {
      return false;
    }
    Shouldn't that be better "if (!obj instanceof ZipCodeInfo) {"
    !obj is always instanceof Boolean, not ZipCodeInfo. Did you mean !(obj instanceof ZipCodeInfo)?

    That would be a synatx error, so it'd be:

    if (!(obj instanceof ZipCodeInfo)) {
  • Vlad (unregistered) in reply to foo AKA fooo

    Actually a prime is a number that's ONLY divisible by itself and 1.

  • Ray (unregistered) in reply to RobFreundlich

    I always thought something was fundamentally wrong with the universe.

Leave a comment on “Immutable is my Name”

Log In or post as a guest

Replying to comment #:

« Return to Article