- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Why is that a WTF? Setters do the same thing... It's a convenient behvior that makes working with properties much easier and intuitive.
Admin
You're conflating local side effects (changing state, the reason Cocktothorpe mentioned) with global side effects (I/O and such).
Admin
Unless the business logic dictates that those actions should occur when the property is set. That's the point of properties; doing things that are related to the property at the time the value is set without the calling code having to worry about the implementation.
Admin
The perfect example of how this could fail was good old IE 5. You could move an object by updating its .x and .y properties. The obvious problem there is that if you update .x and then .y, it should perform two motions. IE being IE, there was some internal kludge to get around this, but the model is fundamentally flawed. Generally, with any kind of GUI library, I want the GUI to appear all at once, properly, not slowly updating itself each time I change a setting.
It doesn't make sense with transactions. If setting a property involves updating a DBMS, it will start a transaction, update the DBMS, and commit the transaction. Then I set the next property and it starts its own transaction. Now if there's an error, only one is rolled back. And, sure, you could manually wrap them in a transaction, but at that point why bother with the side effects?
Admin
Agreed, however I'm speaking to the misuse (and likely misunderstanding) of what properties are and what they are used for. Generally you won't see too many places where a getter or setter cause side effects outside of the containing class. For example, in the SqlCommand object, you wouldn't expect setting the Text property to call ExecuteNonQuery, right?
You set the property state, then call a method to act on the object's state. Take for example ASP.NET Control. If you disassemble it you'll notice the it doesn't set itself to visible right then and there, obviously. And do you know why? Because on Render, the inheriting control will know what to do with Visible = false/true. So you set it's state to Visible=false, then on render that state is acted upon.
Admin
A property holds a portion of the state of an object. Pure and simple. If setting a property causes an email to be sent, data written to the database, etc., that is not object state and so should not be part of the code of the property setter.
I might be convinced (on a case-by-case basis) that the property setter can call a method which does these things. I'd rather see a method that does these things and updates the property as well.
Admin
Since there was no getter and no way to reverse setting the values to read only, having this be a property instead of a function is confusing.
Addendum (2011-05-18 13:09):
Since there was no getter and no way to reverse setting the values to read only, having this be a property instead of a function is confusing. Additionally, it does the same thing regardless of what value you set the "Property" to.
Admin
I agree, but you stated that TRWTF is "write only accessors are effectively just functions". I was asking why that was a WTF...
Admin
Indeed, so restating... TRWTF is write-only accessors.
Admin
Admin
Admin
FTFY
Admin
yawn
damn you akismet
Admin
He should just RTFM (RTF Manuel)
Admin
Not arguing for the "rightness" of this solution in any way, but in a Windows Forms app the driving force behind this could likely be that the desire for the read-only version to use the default appearance of a label and for the read/write version to use the default appearance of a textbox. I have seen code similar to this when .NET newbies didn't know the various appearance properties well enough to make this transition through code.
Captcha: wisi Coding in anything but binary is wisi
Admin
If you want the read-only text to look different than the editable text, this is a reasonable way to do it. As someone else said, you have the label placed on top of the text box and just make visible whichever one you need at the time.
The rest of the code is silly, but not really bad enough to be a WTF, just a "shake your head and move on" level.
Admin
Don't let's start that again.
I literally had too much of it yesterday
Admin
Not justifying the code, but...
Couldn't help but wonder whether one was a label (organizationNameRead), and one was a textbox (organizationName).
A textbox looks greyed out if you set Enabled to false (I assume you can change this, but I've never tried), so putting a label over the top may not be the stupidest idea in the world to make the Company name still look active without being active....
Of course, without context a lot of things are difficult (I don't really know why we'd need an editable company name anyways, but there may be a good reason)
Admin
Duh....the title gives it away.... Some of the article is INVISIBLE!!!
Admin
I better get another coffee....and a new monitor!
Admin
TRWTF is setters and getters. Who needs them when you get access to the class members?
Versus
It's obvious which is better.
Admin
Nearly had me....
Admin
I love it when bunches of "developers" decide whether or not an ambigous hunk of code is a WTF without seeing its use case. It's almost like they imagine they understand all possible situations, and they've never chosen a strange construct on purpose.
Bonus: if you complain about a setter being used too much like a method, I'm not even going to stoop to insult you. You'd probably miss it, anyway. But just in case you feel like taking it personal, please feel free to do so.
Admin
I wonder if the original dev team literally evaporated...
Admin
I'm sure you're trolling but anyway.
There is no use case that would ever justify this ever.
Elsewhere in the code you see this: NameReadonly = false;
or NameReadOnly = readonlyCondition;
or If(readonlyCondition) NameReadOnly = true; else NameReadOnly = false;
They all look reasonable, but they don't work. There are other solutions that are functionally identical and have no potential to be confusing, therefore this 'solution' is strictly worse in every possible way.
I'm a little surprised that nobody mention the TextBox.Readonly property.
Admin
Consultants are the excuse the internal development management use so that they don't get fired.
Before a project fails, hire consultants to take the blame, get promoted before the failure is visible, rinse and repeat.
I can't complain: being used as an excuse for a failed project is part of the service I offer as an external consultant. It's one of the things I get paid for.
Admin
Admin
Um... code smell?
Admin
If you have a hole in the bottom of your ship letting in water, you need a another hole to let it drain out again.
Admin
Another one I just don't get. I prefer the ones with the pictures.
Admin
Oh dear ... I can feel another meme coming on.
Admin
Admin
I really don't get why the code here is worthy of a WTF. They are simply hiding the edit box and showing the read only control. I think the major thing wrong here is poor naming.
Admin
You've all made the incorrect assumption that the fields are both on the same page...