1

Work in C#, want to reduce if else series, entity have two property FromServiceID and ToServiceID ,suppose my ServiceClass instance have bellow information.How to clean up bellow code? any type of suggestion will be acceptable.

entity= new ServiceClass();
entity.FromServiceID=3
entity.ToServiceID=1

if (entity.FromServiceID == 1)
{
    entity.1KWithdrawal();
}
else if (entity.FromServiceID == 2)
{
    entity.10KWithdrawal();
}
else if (entity.FromServiceID == 3)
{
    entity.BTWithdrawal();
}           
if (entity.ToServiceID == 1)
{
    entity.1KDeposit();
}
else if (entity.ToServiceID == 2)
{
    entity.10KDeposit();
}
else if (entity.ToServiceID == 3)
{
    entity.BTDeposit();
}


public class ServiceClass
{ 

    public int FromServiceID { get; set; }
    public int ToServiceID { get; set; }

    public void 1KWithdrawal()
    { Console.WriteLine("One_KWithdrawal"); }

    public void 10KWithdrawal()
    { Console.WriteLine("Ten_KWithdrawal"); }

    public void BTWithdrawal()
    { Console.WriteLine("BTWithdrawal"); }

    public void 1KDeposit()
    { Console.WriteLine("One_KDeposit"); }

    public void 10KDeposit()
    { Console.WriteLine("Ten_KDeposit"); }

    public void BTDeposit()
    { Console.WriteLine("Ten_KDeposit"); }
}
shamim
  • 6,640
  • 20
  • 85
  • 151
  • 5
    Use `switch ... case ...` instead – GSP Feb 28 '17 at 09:47
  • 1
    Is that pseudocode? You are intializing objects but don't assign the instance to a variable, so they are ready to be garbage collected right after initialization. – Tim Schmelter Feb 28 '17 at 09:49
  • 1
    Since when can class names start with a number? – pinkfloydx33 Feb 28 '17 at 09:55
  • We can't help much if we don't know what this class is for and what those methods do. It's too abstract to provide a real improvement. Normally i'd suggest to encapsulate the logic in `ServiceClass`. So implement a method `Process`(or whatever) which calls the methods according to the values of the properties. Then you only need `entity.Process();` – Tim Schmelter Feb 28 '17 at 09:59
  • 1
    Identifiers (such as method names or type names) cannot begin with a digit, so `1KWithdrawal();` cannot be real code. – Jeppe Stig Nielsen Feb 28 '17 at 10:24

4 Answers4

3

Use a Dictionary. Something like this:

Dictionary<int, ServiceClass> dictionary = new Dictionary<int, ServiceClass>()
{
    {1,  new ServiceClass()},
    {2,  new ServiceClass()},
    {3,  new BTWithdrawal()},//assume BTWithdrawal inherits from ServiceClass
};

An example of how using it:

ServiceClass value=new ServiceClass();
value.FromServiceId=1;
value.ToServiceId = 2;
dictionary.TryGetValue(value.FromServiceId, out value);
//or dictionary.TryGetValue(value.ToServiceId, out value);
if (value != null) MessageBox.Show(value.Id.ToString());
Salah Akbari
  • 39,330
  • 10
  • 79
  • 109
  • 2
    The OP checks two different properties though – pinkfloydx33 Feb 28 '17 at 09:55
  • @S.Akbari , thanks for your reply, will you please describe how to use your syntax. – shamim Feb 28 '17 at 09:59
  • @S.Akbari,thanks for your reply,suppose My ServiceClass instance value have information like:ServiceClass value=new ServiceClass();value.FromServiceId=1;value.ToServiceId=2,Now how i used your syntax. – shamim Feb 28 '17 at 10:07
  • @S.Akbari,thank for your valuable effort, please will you tell me why it's better then my syntax. – shamim Feb 28 '17 at 10:14
  • @pinkfloydx33 Check my updated answer. I think the example of how using the dictionary handle this two different properties. – Salah Akbari Feb 28 '17 at 10:14
  • @shamim Well less and more readable code. Also check the documentation https://msdn.microsoft.com/en-us/library/xfhwa508(v=vs.110).aspx – Salah Akbari Feb 28 '17 at 10:15
  • 1
    The use of one variable `value` for both an instance containing input, and for the `out` variable, is not a "canonical" or typical example for `TryGetValue`, so this could leave people who do not know this method in advance, unnecessarily confused. – Jeppe Stig Nielsen Feb 28 '17 at 10:28
  • @S.Akbari,will you please check my update question syntax.How to use it with with your syntax.instead of class initialization want to use method. – shamim Feb 28 '17 at 12:06
1

Maybe this is an overkill, but you can create a class for each one of your cases that inherits from a common interface (let's call it ICommon) that exposes a common method for each case (in your case a Create method) and then inject that interface in the constructor of ServiceClass.

Then when you want to use ServiceClass, you will have to provide an actual implementation of ICommon (one of the classes you extracted from each case) and finally you only have to call entity.Create.

I believe this is the strategy pattern, that in summary says that you should extract an algorithm in a different class under a common interface.

Finally, this refactoring will reduce the cyclotomic complexity of your code (this mainly means that you reduce the branching on your code) which always a good thing.

Mario
  • 313
  • 2
  • 11
0

You can use switch case as below:

var entity = new ServiceClass();

entity.FromServiceID = 3;
entity.ToServiceID = 1;

switch(entity.FromServiceID)
{
    case 1:
        new 1KWithdrawal();
        break;
    case 2:
        new 10KWithdrawal();
        break;
    case 3:
        new BTWithdrawal();
        break;
}

switch(entity.ToServiceID)
{
    case 1:
        new 1KDeposit();
        break;
    case 2:
        new 10KDeposit();
        break;
    case 3:
        new BTDeposit();
        break;
}
Salah Akbari
  • 39,330
  • 10
  • 79
  • 109
MarsRoverII
  • 111
  • 1
  • 15
0

What you could do is to put all the variations into an enum and call the enum values exactly like your methods that you would like to call. (I would suggest not to use numbers in the name, since the compiler won't allow it)

For the sake of simplicity and testability I put the enum and the methods into the same class:

public class ServiceClass
{
    public enum ServiceID
    {
        OneKWithdrawal,
        Ten_KWithdrawal,
        BTWithdrawal,
        OneKDeposit,
        Ten_KDeposit,
        BTDeposit
    }
    public ServiceID From_Ser_ID { get; set; }
    public ServiceID To_Ser_ID { get; set; }

    public void One_KWithdrawal()
    { Console.WriteLine("One_KWithdrawal"); }

    public void Ten_KWithdrawal()
    { Console.WriteLine("Ten_KWithdrawal"); }

    public void BTWithdrawal()
    { Console.WriteLine("BTWithdrawal"); }

    public void One_KDeposit()
    { Console.WriteLine("One_KDeposit"); }

    public void Ten_KDeposit()
    { Console.WriteLine("Ten_KDeposit"); }
}

This would be the method that would execute your if-condition methods. It uses reflection to access the methods that are coded in the enum. You probably have to adjust the object parameter in the Invoke(sc, null); call depending on where your methods are situated. If they are in the same class as where you would call execute you can use this.

public static void execute(ServiceClass sc)
{
    sc.GetType().GetMethod(sc.From_Ser_ID.ToString()).Invoke(sc, null);
    sc.GetType().GetMethod(sc.To_Ser_ID.ToString()).Invoke(sc, null);
}

And here you can test the entire code:

public static void Main(string[] args)
{

    ServiceClass entity = new ServiceClass();
    entity.From_Ser_ID = ServiceClass.ServiceID.BTWithdrawal;
    entity.To_Ser_ID = ServiceClass.ServiceID.Ten_KDeposit;

    execute(entity);
}

So you would end up with an enum and 2 lines of code.

Mong Zhu
  • 23,309
  • 10
  • 44
  • 76