Comment On Let's Accessorize

fist

Um, "get" returns the return-code of the "SAVE-to-database" action?

Um, "set" returns the return-code of "LOAD-from-???" action?

Aside from the non-trivial processing wtf, wouldn't those make more sense if they were reversed (eg: "set" stores the value in the DB, and "get" retrieves it)?

frist

codeman:

Um, "get" returns the return-code of the "SAVE-to-database" action?

Um, "set" returns the return-code of "LOAD-from-???" action?

Aside from the non-trivial processing wtf, wouldn't those make more sense if they were reversed (eg: "set" stores the value in the DB, and "get" retrieves it)?

I would agree.

First

PCBiz:

codeman:

Um, "get" returns the return-code of the "SAVE-to-database" action?

Um, "set" returns the return-code of "LOAD-from-???" action?

Aside from the non-trivial processing wtf, wouldn't those make more sense if they were reversed (eg: "set" stores the value in the DB, and "get" retrieves it)?

I would agree.

I seriously wonder what the hell the author of this was thinking....

Also, how long until we get the auto-ip-ban script when someone posts 'first' or some various misspelling?

So, accessing the address fields does not save to the db, while accessing the rest does? I don't get it.

This must be such a WTF that I'm not even really following it. 
The point of the class is to just wrap the setting and getting of the
non-object variables?  Seems the author doesn't understand object
property manipulation insomuch as objects in general...

PCBiz:

codeman:

Um, "get" returns the return-code of the "SAVE-to-database" action?

Um, "set" returns the return-code of "LOAD-from-???" action?

Aside from the non-trivial processing wtf, wouldn't those make more sense if they were reversed (eg: "set" stores the value in the DB, and "get" retrieves it)?

I would agree.

Actually, properties are usually wrapped around private variables.  The "setter" refers to the routine that takes a value and "sets" that private variable to that value.  The "getter" refers to the routine that returns the value of the private variable.

The property pattern is actually pretty useful, and is definitely not the wtf here.  Often, when working in ASP.NET, I have the setter set not only the variable but also update the ViewState.  The problem here is that the property has a major side effect.  Code calling these properties would look like Contact.contact = theContact, which looks completely innocent.

Russ should be thanking his sweet job that the getter function didn't say "DeleteDatabase()".

 

Anonymous:

So, accessing the address fields does not save to the db, while accessing the rest does? I don't get it.

I see your problem: you don't get it because you are thinking coherently! Bang your head with a hammer a few times and look at it again, it'll start to make sense [8o|]

Alex Papadimoulis:

public Contact contact 

{

  get 

  { 

    return SaveToDatabase();

  }

  set

  {

    LoadFromContact(value);

  }

}

I have to know...  is this an obfuscation error, or were the methods truly implemented in this way? 

This is horrible on so many levels.

P.S. Fourteenth!!

There's really no sense in using functions when you can just
use an accessor is there?  Doing it this way eliminates the need for all
those annoying parenthesis throughout your code.   The real WTF is........okay so it is the code after all

bullseye:

Alex Papadimoulis:

public Contact contact 

{

  get 

  { 

    return SaveToDatabase();

  }

  set

  {

    LoadFromContact(value);

  }

}

I have to know...  is this an obfuscation error, or were the methods truly implemented in this way? 

This is horrible on so many levels.

P.S. Fourteenth!!

Judging by the comments after some of the lines in the first snippet this code was truly implemented that way.  Yikes!

Alex Papadimoulis:

//ED: From the Contact class ...

/// 


/// Gets or sets the contact passed to it.

/// This hasn't neccessarily been saved to the database 

/// 

public Contact contact 

{

  get 

  { 

    return SaveToDatabase();

  }

  set

  {

    LoadFromContact(value);

  }

}

This is nonsensical.  The accessor methods appear to do exactly the opposite of the usual meaning.  I'm guessing this is somewhere on a government website.  And hey, come to think of it, it's pretty enterprise too.  You can define the accessor methods to do whatever the hell you want them to do, good programming practices be damned!

dmdietz:

Alex Papadimoulis:

//ED: From the Contact class ...

/// 


/// Gets or sets the contact passed to it.

/// This hasn't neccessarily been saved to the database 

/// 

public Contact contact 

{

  get 

  { 

    return SaveToDatabase();

  }

  set

  {

    LoadFromContact(value);

  }

}

This is nonsensical.  The accessor methods appear to do exactly the opposite of the usual meaning.  I'm guessing this is somewhere on a government website.  And hey, come to think of it, it's pretty enterprise too.  You can define the accessor methods to do whatever the hell you want them to do, good programming practices be damned!

 

So, for example, one could write (not sure if this is the right way to format stuff for the forum s/w):

<pre>public Contact contact {

   // Misuse get using WPP (worst-programming-practices)

   get { return True || Boolean.FALSE.toString().equals("TrUe")  && FileNotFound; }

   // Misuse set with implied side effect

   set { wtf(); }

}</pre>

 

Damn, it was the wrong way to format it. Can anyone provide a link to something that describes the proper way to format a post?

Thanks in advance!

I like that the comment on the accessor claims that this hasnt been saved to the database. Which it does.  Everytime we access the object.

bullseye:

Alex Papadimoulis:

public Contact contact 

{

  get 

  { 

    return SaveToDatabase();

  }

  set

  {

    LoadFromContact(value);

  }

}

I have to know...  is this an obfuscation error, or were the methods truly implemented in this way? 

This is horrible on so many levels.

P.S. Fourteenth!!

No it does make sence in an extreamly disturbing way, note that the code came from the Contact class. So you end up with something like:

Contact c = new Contact();
c.contact = new Contact(1234); // Populate the contact c from the database row with 1234
Contact newContact = c.contact; // Save the contacts state to the database populating the ID

Still an enourmouse WTF but I can see how they arived at this pattern. It would have made more sence if the type the accessors accept / return was an ID rather than a contact. Still would have been a stupid thing to do though.

The real WTF here is that the tag isn't used here for xmldoc'ing.

Let's try &lt;value&gt;&lt;/value&gt; instead. DOH.

Alex Papadimoulis:

public Contact contact 

{

  get 

  { 

    return SaveToDatabase();

  }

  set

  {

    LoadFromContact(value);

  }

}

It's amazing when I run into code like this that causes no end to problems when users mistakingly reference the property from within the methods they execute from within the property.  I guess school's don't teach looping referential bugs in code these days.

This definitely puts the OO in mOOse ( m Object-Oriented s e ).  I'll leave the "m", "s", and "e" as an exercise for the reader. [|-)]

The scary thing here is what happens when you debug it and the debugger automatically call the property to get its value.
Can you smell a heisenbug?

Anonymous:

No it does make sence in an extreamly disturbing way, note that the code came from the Contact class. So you end up with something like:

Contact c = new Contact();
c.contact = new Contact(1234); // Populate the contact c from the database row with 1234
Contact newContact = c.contact; // Save the contacts state to the database populating the ID

Still an enourmouse WTF but I can see how they arived at this pattern. It would have made more sence if the type the accessors accept / return was an ID rather than a contact. Still would have been a stupid thing to do though.

Ugh...  I think you're right.  Kudos for decoding that monstrosity.

On a side note, this code is causing me to have bitter beer face.

The real WTF:

Anonymous:

frist

first post, w00t!!

Heh, bind it to a datagrid with a few users and have a look at the dbserver-load [:O]

bullseye:

Anonymous:

No it does make sence in an extreamly disturbing way, note that the code came from the Contact class. So you end up with something like:

Contact c = new Contact();
c.contact = new Contact(1234); // Populate the contact c from the database row with 1234
Contact newContact = c.contact; // Save the contacts state to the database populating the ID

Still an enourmouse WTF but I can see how they arived at this pattern. It would have made more sence if the type the accessors accept / return was an ID rather than a contact. Still would have been a stupid thing to do though.

Ugh...  I think you're right.  Kudos for decoding that monstrosity.

On a side note, this code is causing me to have bitter beer face.

1 or 2 or 15 keystone lights would help you out there, my man.  It's the never bitter beer.  :)

Well, it just looks to me like a write-on-access pattern, i.e you can
set the contact, but it's only persisted if it's ever used.  I'd
guess the idea is that you pass in an existing contact to clone it, and
that clone is only saved to the database if it ends up being used
somewhere.  It's similar to copy-on-write.



There isn't enough code here to decide if it's a real WTF,
SaveToDatabase() might have a 'dirty' flag set by LoadFromContact that
let it decide if the contact does in fact need to be written or
not.  Other properties might call SaveToDatabase when they are
modified, etc.

Digitalbath:

bullseye:

Anonymous:

No it does make sence in an extreamly disturbing way, note that the code came from the Contact class. So you end up with something like:

Contact c = new Contact();
c.contact = new Contact(1234); // Populate the contact c from the database row with 1234
Contact newContact = c.contact; // Save the contacts state to the database populating the ID

Still an enourmouse WTF but I can see how they arived at this pattern. It would have made more sence if the type the accessors accept / return was an ID rather than a contact. Still would have been a stupid thing to do though.

Ugh...  I think you're right.  Kudos for decoding that monstrosity.

On a side note, this code is causing me to have bitter beer face.

1 or 2 or 15 keystone lights would help you out there, my man.  It's the never bitter beer.  :)

Awesome:  http://en.wikipedia.org/wiki/Keystone_Light

 

TomCo:

This definitely puts the OO in mOOse ( m Object-Oriented s e ).  I'll leave the "m", "s", and "e" as an exercise for the reader. [|-)]

Object Oriented

or

Objectionably Obtuse?

Digitalbath:

Digitalbath:

bullseye:

Anonymous:

No it does make sence in an extreamly disturbing way, note that the code came from the Contact class. So you end up with something like:

Contact c = new Contact();
c.contact = new Contact(1234); // Populate the contact c from the database row with 1234
Contact newContact = c.contact; // Save the contacts state to the database populating the ID

Still an enourmouse WTF but I can see how they arived at this pattern. It would have made more sence if the type the accessors accept / return was an ID rather than a contact. Still would have been a stupid thing to do though.

Ugh...  I think you're right.  Kudos for decoding that monstrosity.

On a side note, this code is causing me to have bitter beer face.

1 or 2 or 15 keystone lights would help you out there, my man.  It's the never bitter beer.  :)

Awesome:  http://en.wikipedia.org/wiki/Keystone_Light

 

 

And I thought a Keystone Light was a device to cast an irregular pattern of illumination!

So is the WTF here supposed to be (aside from the switched logic of the get/set functions) that they do more than just obtain and modify some internal property and instead initiate some database transaction? What's wrong with a database abstraction layer?

Okay, I've been thinking about this for about 15 minutes now, and I think I've figured out what the original developer was doing.

First of all, it doesn't make sense to give a Contact a property called "contact" that is also a Contact.  If we have a Contact bob, then I can understand who bob.mother and bob.father are, but who is bob.contact?

Well, I guess it is this guy's way of saving and loading bob to/from the database!  Example:

Contact bob = new Contact();
bob.contact = "Bob"; // load contact identified by "Bob" from database
// modify bob here
Contact dummy = bob.contact; // save bob back to database

Why he didn't just make SaveToDatabase() and LoadFromContact() public, we may never know.

what's wrong with this?

this is what consultants do!... very interprisey indeed

Assuming my interpretation is correct, and that bob.contact == bob, the first code fragment could also be written as:

// none of the following will save
string firstName = contactDetails.firstName;
string lastName = contactDetails.lastName;
string address1 = contactDetails.address1; 
string address2 = contactDetails.address2;
string primaryPhone = contactDetails.primaryPhone;

Why address1 and address2 do not save is beyond me.  Maybe they really do save, but whoever added those lines didn't bother putting the "saves" comment.

That is indeed one bass-ackwards pattern, but I can kinda see it. I honestly can't see why you'd need such a pattern, other than to be sure that it saves the data to the database everytime anybody modifies it in certain ways. And the set is completely baffling. Yes, you could do bob.contact = "Bob" to load Bob's info in from the database (or wherever), but why you'd want to do it in that particular way is a bit beyond me.

I find that it's hard to say whether the author is an absolute genius or a complete gibbering idiot. Either one of them could have written this code. :)

Gentlemen, I see you've all become acquainted with our new code obfuscator.  We stymie your competition by secretly switching the contents of your getters and setters with totally random functions.  And Folger's Crystals  

Otto:

And the set is completely baffling. Yes, you could do bob.contact = "Bob" to load Bob's info in from the database (or wherever), but why you'd want to do it in that particular way is a bit beyond me.

Looking at it again, and at the comments above the property, I think I was wrong.  Now I'm pretty sure the parameter for LoadFromContact is supposed to be another Contact.  My best guess is that it actually copies the data from the passed Contact to this, as opposed to loading the data from the database.

Who knows, maybe there is actually some use for always saving (or saving only if there have been changes) whenever a Contact is used :P.

Anonymous:

PCBiz:

codeman:

Um, "get" returns the return-code of the "SAVE-to-database" action?

Um, "set" returns the return-code of "LOAD-from-???" action?

Aside from the non-trivial processing wtf, wouldn't those make more
sense if they were reversed (eg: "set" stores the value in the DB, and
"get" retrieves it)?

I would agree.

I seriously wonder what the hell the author of this was thinking....

Also, how long until we get the auto-ip-ban script when someone posts 'first' or some various misspelling?

Oh, come on.  Thanks to the latest misspelling the ubiquitous "first" post can now be "Senate Majority Leader!"

Anonymous:

frist

Boobies!

Also, I don't want to see the interfaces for that monstrosity.

Anonymous:

The scary thing here is what happens when you debug it and the debugger automatically call the property to get its value.
Can you smell a heisenbug?

Exactly what I was thinking. In fact, I'm going to bookmark this page so I have it handy next time I need to explain what a heisenbug is - I don't know when I've ever seen a finer specimen.

Anonymous:

the switched logic of the get/set functions...

void Contact::LoadFromContact(Contact foo)
{
    firstName = foo.firstName;
    lastName = foo.lastName;
    ...etc...
}

The copy constructor probably calls LoadFromContact too.

As another poster said, the getter implements a write-on-access scheme.  I don't know why so many people think the logic is switched.

I agree 'bout parentheses being annoying, but I can at least think of one aesthetic reason to use a Property where a Function would otherwise do; Functions grouped in the Methods section section of the Intellisense thingy (instead of with other Properties), and sometimes having consistent & reliable visual cues on things like that can make a larger than expected difference in productivity on a project.

There is absolutely nothing perverse about this save for the fact that OO programmers have so forgotten how to do OO that they think a getter is a proper part of OO programming. What is the point of a getter? you declare a field private and then expose it immediately as public thru a getter/setter pair? Ridiculous, just declare it public.

The procedural people (read C and Basic) have infiltrated and messed up OO so much that they actually have you believing that it is perverse for an object to encapsulate a method like retrieving a value from a database. And to the people who say "a getter is useful to do a bit more before returning the value" you've either forgotten what interfaces are meant for or are ignoring the fact that 95% of all getters written out there do NOTHING but return the value (or both). Even the most ardent of the getter apologists will admit that. And before you go saying that only bad programmers write empty getters, take a look thru the last bean you wrote.

Think how silly it would look if you wrote:

private public int x;

now think about what accessors and mutators do. C# programmers are somewhat off the hook because of properties, but if you look under the hood its just syntactic sugar and essentially does the same thing as a java/bean getter. Shame on you.

Going to flame? Read this article first:
http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html

I have to agree that there is
nothing wrong with getter and setter methods actually doing something,
including persisting the database. As to other expensive computation,

I belive that is called lazy evaluation, and is definetly not a WTF.

agreed, but a small correction: lazy initialization not lazy evaluation. lazy init is lazily fetching portions of an object graph not immediately needed (which hibernate actually uses dynamic proxies for so it doesnt even really need getters).

lazy eval is a language feature which delays evaluation of an expression until it is needed. java does not support this (java is strict-evaluative), neither does c# or any other imperative/OO language.

but yea, rock on kornad

"the proper way to format a post"?  Surely you jest!

But serisouly, I've found the best way to get the formatting right is to

FUCK THE RICH EDITOR IN THE ASS

and just use the HTML editor.  Surround code and whatnot in <pre> tags and use <br> tags to insert "newlines".  If you want to be sure, copy the HTML to a file and open it up in a browser and see how it renders.

Well, that's what I get for trying to use the rich text editor.  The text in the black box is supposed to be large and red, but I think I actually like it better the way it came out.  Highlight the black box for a surprise!