is it possible to do this without using an "if" (asp.net mvc post action method)

Question

according to anti-if campaign it is a best practice not to use ifs in our code. Can anyone tell me if it possible to get rid of the if in this piece of code ?

 [AcceptVerbs(HttpVerbs.Post)]
 public ActionResult Create(OrganisationInput organisationInput)
 {
    if (!this.ModelState.IsValid)
    {
       return View(organisationInput.RebuildInput<Organisation, OrganisationInput>());
    }

   var organisation = organisationInput.BuildEntity<Organisation, OrganisationInput>();
   this.organisationService.Create(organisation);

   return this.RedirectToAction("Index");
}

Answer 1

Looks like a valid use for "if".

The anti-if campaign appears to be against the abuse of if statments (i.e. too many nested ifs) etc, not for complete eradication of them.

They are Necessary.

Answer 2

Use a ternary operator :-) It's worse, but it's not an if.

Answer 3

It is a valid if in this case, as it can only be one of two states. The anti-if campaign seems more geared to the endlessly open-ended if/else/switch statments. Looks like a good campaign!

Answer 4

Since you are asking: Yes, it is possible.

Create an abstract class A with an abstract method that returns ActionResult, created via a factory method that uses a dictionary of Func's that creates A-subclassed instances based on Controller state computed from model (in your case, from the controllerInstance.Model.IsValid property value).

Sample code:

public abstract class ModelStateValidator
{
  private Controller controller;

  protected Controller Controller {
    get { return controller; }
  }

  public abstract ActionResult GetResult();

  #region initialization
  static ModelStateValidator() {
    creators[ControllerState.InvalidModel] = () => new InvalidModelState();
    creators[ControllerState.ValidModel] = () => new ValidModelState();
  }
  #endregion
  #region Creation
  private static Dictionary<ControllerState, Func<ModelStateValidator>> creators = new Dictionary<ControllerState, Func<ModelStateValidator>>();

  public static ModelStateValidator Create(Controller controller) {
    return creators[GetControllerState(controller)]();
  }

  private static ControllerState GetControllerState(Controller c) {
    return new ControllerState(c);
  }

  internal class ControllerState
  {
    private readonly Controller controller;
    private readonly bool isModelValid;

    public ControllerState(Controller controller)
      : this(controller.ModelState.IsValid) {
      this.controller = controller;
    }

    private ControllerState(bool isModelValid) {
      this.isModelValid = isModelValid;
    }

    public static ControllerState ValidModel {
      get { return new ControllerState(true); }
    }

    public static ControllerState InvalidModel {
      get { return new ControllerState(false); }
    }

    public override bool Equals(object obj) {
      if (obj == null || GetType() != obj.GetType())  //I can show you how to remove this one if you are interested ;)
      {
        return false;
      }

      return this.isModelValid == ((ControllerState)obj).isModelValid;
    }

    public override int GetHashCode() {
      return this.isModelValid.GetHashCode();
    }
  }

  #endregion
}

class InvalidModelState : ModelStateValidator
{
  public override ActionResult GetResult() {
    return Controller.View(organisationInput.RebuildInput<Organisation, OrganisationInput>());
  }
}

class ValidModelState : ModelStateValidator
{
  public override ActionResult GetResult() {
    return this.Controller.RedirectToAction("Index");
  }
}

80 additional lines, 4 new classes to remove a single if.

your usage then, instead of if, calls the method like this:

ActionResult res =  ModelStateValidator.Create(this).GetResult();

NOTE: Of course it should be tweaked to acommodate the code that is between the ifs in your original question, this is only a sample code :)

---

Adds additional unnecessary complexity? YES.

Contains ifs? NO.

Should you use it? Answer that yourself :)

Answer 5

You could just use a While loop... but that seems like a glorified if statement.

 private static bool Result(bool isValid)
        {
            while(!isValid)
            {
                return true;
            }

            return false;
        }