3
\$\begingroup\$

I have following code that is used of one time skipping some functionality(actually do something only one time until SkipSomeStuff is true):

private bool _skipSomeStuff;
public bool SkipSomeStuff
{
  set 
  { 
    _skipSomeStuff = value; 
  }
  get 
  { 
    if (_skipSomeStuff) 
    {
        _skipSomeStuff = false;
        return true;        
    }
    return false;
  }
}

Is such construction ok to use, or I should change it with something?

\$\endgroup\$
10
\$\begingroup\$

It is very confusing to set a property just to discover that it returns something else just after you set it. Properties are supposed to keep the value you give them. (At least logically)

And since your SkipSomeStuff property is public, you have no guarantee you won't access that property by accident before the intended recipient accesses it, and the stuff isn't skipped after all.

How about keeping the skipSomeStuff member, but set it using a method called SkipStuffNextTime() or something like that? Nobody will misunderstand the intent of your code that way, and only the code it's relevant for will access and reset it.

private bool skipSomeStuff = false;

public void SkipStuffNextTime() {
    skipSomeStuff = true;
}

public void DoSomeStuff() {
    if (skipSomeStuff) {
        skipSomeStuff = false;
        return;
    }
    // do the stuff
}
\$\endgroup\$
  • \$\begingroup\$ +1 for the plug for maintainability. P.S. A certain property that emits a value different from the underlying field - but never updates the field itself! This damn thing has wreaked havoc in our application. And guess what you see in the debugger when you hover over the property? Yes, the wrong answer (i.e. wrong value). \$\endgroup\$ – radarbob Jan 24 '12 at 2:56
  • 1
    \$\begingroup\$ @radarbob - I imagine that if you accidentally hover your mouse over that property at the wrong time while debugging, you might inadvertently "use up" the value, causing your application to run incorrectly. Nasty problem. \$\endgroup\$ – Dr. Wily's Apprentice Jan 24 '12 at 8:24
  • \$\begingroup\$ @Dr.Wily'sApprentice interesting suggestion. I'll try) \$\endgroup\$ – anatoliiG Jan 24 '12 at 9:06
  • \$\begingroup\$ @Lars-Erik thanks for answering. I'll refactor this crap) \$\endgroup\$ – anatoliiG Jan 24 '12 at 9:07
  • \$\begingroup\$ :) If you add an accessor property (only getter), you can even write some unit-tests which confirms that everything works as expected. \$\endgroup\$ – Lars-Erik Jan 24 '12 at 11:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.