Return to Answer

Update

As I said in my comments, complex logic doesn't really change anything.

You want to process a disputed order. There are multiple ways of doing that. Disputed order type can be Enum:

public void ProcessDisputedOrder(DisputedOrder order)
{
   switch (order.Type)
   {
       case : DisputedOrderType.Canceled
          var strategy = new StrategyForDisputedCanceledOrder();
          strategy.Process(order);  
          break;
       case : DisputedOrderType.LessThan7Days
          var strategy = new DifferentStrategy();
          strategy.Process(order);
          break;
       default: 
          throw new NotImplementedException("");
   }
}

There are many ways of doing this. You can have inheritance hierarchy of Order, DisputedOrder, DisputedOrderLessThan7Days, DisputedOrderCanceled etc. This is not nice, but it would also work.

Original Answer (no longer relevant as I thought you were after something simpler)

CheckBannedIP(login.IP);
CheckLoginTrial(login);

Authenticate(login.Username, login.Password);
 

public void CheckBannedIP(string ip)
{
    // If banned then re-direct, else do nothing.
}

public void CheckLoginTrial(LoginAttempt login)
{
    // If exceeded trials, then inform user, else do nothing
}

public void Authenticate(string username, string password)
{
     // Attempt to authenticate. On success redirect, else catch any errors and inform the user. 
}

As I said in my comments, complex logic doesn't really change anything.

You want to process a disputed order. There are multiple ways of doing that. Disputed order type can be Enum:

public void ProcessDisputedOrder(DisputedOrder order)
{
   switch (order.Type)
   {
       case DisputedOrderType.Canceled:
          var strategy = new StrategyForDisputedCanceledOrder();
          strategy.Process(order);  
          break; 

       case DisputedOrderType.LessThan7Days:
          var strategy = new DifferentStrategy();
          strategy.Process(order);
          break; 

       default: 
          throw new NotImplementedException();
   }
}

There are many ways of doing this. You can have inheritance hierarchy of Order, DisputedOrder, DisputedOrderLessThan7Days, DisputedOrderCanceled, etc. This is not nice, but it would also work.

Original Answer, no longer relevant as I thought you were after something simpler:

CheckBannedIP(login.IP);
CheckLoginTrial(login);

Authenticate(login.Username, login.Password);

public void CheckBannedIP(string ip)
{
    // If banned then re-direct, else do nothing.
}

public void CheckLoginTrial(LoginAttempt login)
{
    // If exceeded trials, then inform user, else do nothing
}

public void Authenticate(string username, string password)
{
     // Attempt to authenticate. On success redirect, else catch any errors and inform the user. 
}

I see the following concerns here:

• Check for banned IP. Checks whether user's IP is in a banned IP range. You will execute this no matter what.
• Check if trials exceeded. Checks whether user have exceeded their login attempts. You will execute this no matter what.
• Authenticate user. Attempt to authenticate user.

I would do the following:

CheckBannedIP(login.IP);
CheckLoginTrial(login);

Authenticate(login.Username, login.Password);


public void CheckBannedIP(string ip)
{
    // If banned then re-direct, else do nothing.
}

public void CheckLoginTrial(LoginAttempt login)
{
    // If exceeded trials, then inform user, else do nothing
}

public void Authenticate(string username, string password)
{
     // Attempt to authenticate. On success redirect, else catch any errors and inform the user. 
}

Currently your example has too many responsibilities. All I did, was encapsulate those responsibilities within methods. Code looks cleaner and you don't have condition statements all over the place.

Factory encapsulates construction of objects. You don't need to encapsulate construction of anything in your example, all you need to do is to separate your concerns.

Update

This would look at the order type and give you a correct strategy for that type of order.

Update

This would look at the order type and give you a correct strategy for that type of order.

You might end up with something on the lines of:

public void ProcessDisputedOrder(DisputedOrder order)
{
   var strategy = DisputedOrderStrategyFactory.Instance.Build(order.Type);   
   strategy.Process(order);
}

Original Answer (no longer relevant as I thought you were after something simpler)

I see the following concerns here:

• Check for banned IP. Checks whether user's IP is in a banned IP range. You will execute this no matter what.
• Check if trials exceeded. Checks whether user have exceeded their login attempts. You will execute this no matter what.
• Authenticate user. Attempt to authenticate user.

I would do the following:

CheckBannedIP(login.IP);
CheckLoginTrial(login);

Authenticate(login.Username, login.Password);


public void CheckBannedIP(string ip)
{
    // If banned then re-direct, else do nothing.
}

public void CheckLoginTrial(LoginAttempt login)
{
    // If exceeded trials, then inform user, else do nothing
}

public void Authenticate(string username, string password)
{
     // Attempt to authenticate. On success redirect, else catch any errors and inform the user. 
}

Currently your example has too many responsibilities. All I did, was encapsulate those responsibilities within methods. Code looks cleaner and you don't have condition statements all over the place.

Factory encapsulates construction of objects. You don't need to encapsulate construction of anything in your example, all you need to do is to separate your concerns.

I see the following concerns here:

• Check for banned IP. Checks whether user's IP is in a banned IP range. You will execute this no matter what.
• Check if trials exceeded. Checks whether user have exceeded their login attempts. You will execute this no matter what.
• Authenticate user. Attempt to authenticate user.

I would do the following:

CheckBannedIP(login.IP);
CheckLoginTrial(login);

Authenticate(login.Username, login.Password);


public void CheckBannedIP(string ip)
{
    // If banned then re-direct, else do nothing.
}

public void CheckLoginTrial(LoginAttempt login)
{
    // If exceeded trials, then inform user, else do nothing
}

public void Authenticate(string username, string password)
{
     // Attempt to authenticate. On success redirect, else catch any errors and inform the user. 
}

Currently your example has too many responsibilities. All I did, was encapsulate those responsibilities within methods. Code looks cleaner and you don't have condition statements all over the place.

Factory encapsulates construction of objects. You don't need to encapsulate construction of anything in your example, all you need to do is to separate your concerns.

I see the following concerns here:

• Check for banned IP. Checks whether user's IP is in a banned IP range. You will execute this no matter what.
• Check if trials exceeded. Checks whether user have exceeded their login attempts. You will execute this no matter what.
• Authenticate user. Attempt to authenticate user.

I would do the following:

CheckBannedIP(login.IP);
CheckLoginTrial(login);

Authenticate(login.Username, login.Password);


public void CheckBannedIP(string ip)
{
    // If banned then re-direct, else do nothing.
}

public void CheckLoginTrial(LoginAttempt login)
{
    // If exceeded trials, then inform user, else do nothing
}

public void Authenticate(string username, string password)
{
     // Attempt to authenticate. On success redirect, else catch any errors and inform the user. 
}

Currently your example has too many responsibilities. All I did, was encapsulate those responsibilities within methods. Code looks cleaner and you don't have condition statements all over the place.

Factory encapsulates construction of objects. You don't need to encapsulate construction of anything in your example, all you need to do is to separate your concerns.

Update

As I said in my comments, complex logic doesn't really change anything.

You want to process a disputed order. There are multiple ways of doing that. Disputed order type can be Enum:

public void ProcessDisputedOrder(DisputedOrder order)
{
   switch (order.Type)
   {
       case : DisputedOrderType.Canceled
          var strategy = new StrategyForDisputedCanceledOrder();
          strategy.Process(order);  
          break;
       case : DisputedOrderType.LessThan7Days
          var strategy = new DifferentStrategy();
          strategy.Process(order);
          break;
       default: 
          throw new NotImplementedException("");
   }
}

There are many ways of doing this. You can have inheritance hierarchy of Order, DisputedOrder, DisputedOrderLessThan7Days, DisputedOrderCanceled etc. This is not nice, but it would also work.

In my example above I look at order type and get relevant strategy for that. You could encapsulate that process into a factory:

var strategy = DisputedOrderStrategyFactory.Instance.Build(order.Type);

This would look at the order type and give you a correct strategy for that type of order.