views:

119

answers:

5

This is similar to :

http://stackoverflow.com/questions/2908876/net-bool-vs-enum-as-a-method-parameter

but concerns returning a bool from a function in some situations.

e.g. Function which returns bool :

    public bool Poll()
    {
        bool isFinished = false;

        // do something, then determine if finished or not.

        return isFinished;
    }

Used like this :

        while (!Poll())
        {
            // do stuff during wait.
        }

Its not obvious from the calling context what the bool returned from Poll() means. It might be clearer in some ways if the "Poll" function was renamed "IsFinished()", but the method does a bit of work, and (IMO) would not really reflect what the function actually does. Names like "IsFinished" also seem more appropriate for properties. Another option might be to rename it to something like : "PollAndReturnIsFinished" but this doesn't feel right either.

So an option might be to return an enum. e.g :

    public enum Status
    {
        Running,
        Finished
    }  

    public Status Poll()
    {
        Status status = Status.Running;

        // do something, then determine if finished or not.

        return status;
    }

Called like this :

        while (Poll() == Status.Running)
        {
            // do stuff during wait.
        }

But this feels like overkill. Any ideas ?

+1  A: 

If you have more than 2 states, use an enum, else just use a bool.

Edit:

As your example, you can easily make use of both, if needed.

public bool IsRunning  { get {return Poll() == Running; }}
public bool IsFinished { get {return Poll() == Finished; }}
leppie
You shouldn't run a long function in a property like that. If you really need it you should set a local field within Poll() and return that in the property.
Jouke van der Maas
@Jouke van der Maas: I have no idea about the implementation details, so I only go on what was given.
leppie
@leppie - you can assume the "Poll" method is relatively expensive to call (e.g. call over a network). Sorry if I was not clear.
Moe Sisko
@Moe Sisko: `Poll` is a terrible name choice then. Something like `QueryStatus` might be more intuitive.
leppie
+2  A: 

First of all code is for people to read, and in your case the enum version is more readable than the bool version.

Edit:

Other advantage of enum version is that you can easily add other statuses if you need. Like Error for example.

bniwredyc
+2  A: 

I follow the .Net convention that boolean properties are prefixed with "Is" and boolean methods are prefixed with "Try" (or "Is" where appropriate).

In your case I think the problem is in the "Poll" name. Name the method stating what it is doing or polling for. e.g. TryDoSomething()

adrianm
+2  A: 

A method should be read like a verb, and the result of the bool Poll() method is misleading, and this is probably why it feels awkward to use.

// you wrote.
while( !Poll() )
{
    // still waiting .. do something.
}

When I first read your code, I thought it said While (the system is) not polling, do something?

But it really says ... Poll, and if not finished polling do something while we wait.

Your enum version appears to have changed the semantics of the call, but for the better, which is why people like it. While Poll() is still Running, do something while we wait.

The most readable code wins.

Robert Paulson
A: 

I read and re-read what you are trying to do. To me, Poll() should mean exactly that you are polling for something. Then I would check the status if it is still running.

My resultant code would look like this.

while (myObject.Poll() && myObject.IsRunning)
{
   // myObject successfully poll it .. 
   // and is successful
   // do more things here... 
}

Note: IsRunning is a getter/setter. Would this be clearer?

Syd