Method that execute action and return boolean

Question

I'm trying to do a connection between a local dataset and a remote dataset . When I run my program, the method below is called.

 public void CheckCanDo()
 {
    if (LocalAcess.HasNonSyncedItems())
     {
       if (RemoteAcess.TryToSync())
       {
          LocalAcess.RefreshDateLastConnection();
          CanDoThing = true;
       } 
       else
       {
           if(LocalAcess.HasExpired)
           {
               CanDoThing = false;
               return;
           }
       }                
     }
     CanDoThing = true;
 }

Is it appropriate to have a method executing actions and returning a boolean as a way to control the application's flow? Does it violate some good practice of C#? Is there a better way?

You're *not* returning a boolean. You probably should be, but you're not. — Servy, Mar 20 '14 at 16:00

score 0 · Answer 1 · answered Mar 20 '14 at 16:18

0

Replace "void" with "bool" before "CheckCanDo()" and instead of setting variables etc, to return a result; just type return a bool, e.g. "return condition;". That's really a better way to do it.

answered Mar 20 '14 at 16:18

score 0 · Answer 2 · answered Mar 20 '14 at 16:29

public bool CheckCanDo()
{
    if (LocalAcess.HasNonSyncedItems())
    {
        if (RemoteAcess.TryToSync())
        {
            LocalAcess.RefreshDateLastConnection();
            return  true;
        }
        else
        {
            if (LocalAcess.HasExpired)
            {
                return false;
            }
        }
    }
    return true;
}

score 0 · Answer 3 · edited May 23 '17 at 12:15

You could try this ... I will advice you to keep business logic strait and to remove to much if- else it could convert business logic into a mess!!!

public bool CheckCanDo()
{
        if (!LocalAcess.HasNonSyncedItems())
        {
            return false;
        }

        // It might be !RemoteAcess.TryToSync() || LocalAcess.HasExpired
        // I did not really understand that part
        if (!RemoteAcess.TryToSync() && LocalAcess.HasExpired)
        {
            return false;            
        }

        LocalAcess.RefreshDateLastConnection();
        return true;             
}

This blog might help you on this more... How to avoid "if" chains?

Method that execute action and return boolean

3 Answers3