CODEWITHSUNDEEP
c#asp.net.netc#-4.0
user1400915
user1400915

Reputation: 1943

Refactoring the code and developing a clean code

I have a dto as follows:

public class DaysDetails
{
 public bool Sun {get;set;}
 public bool Mon {get;set;}
 ...
 public bool Sat {get;set;} //All 7 days of the week
}

I have a method which checks if the days are checked and builds a comma separated string format . For eg: if sunday and monday are checked then the output is "0,1" (numbers corresponding to days)

pubic string ConstructDays(DaysDetails d)
{
 StringBuilder constructDays = new StringBuilder();
 if(d.Sun == true)
  { 
    constructDays.Append("0");
  }
 if(d.Mon == true)
 {
   constructDays.Append("1");
  }
 ..... //So on for all seven days
 string day = Convert.toString(constructDays);

 if(day != string.Empty && day[0] == ",")
   day = day.Remove(0,1);
 return day;
}

I need to convert this function to a more maintainable code and simplified version . What can be improved in this?

Upvotes: 0

Views: 104

Answers (4)

izzy255
izzy255

Reputation: 21

I would suggest two things: make a separate method for converting a boolean to a int representation, and override the ToString method instead of generating a separate ConstructDays method.

public class DaysDetails
{
    public bool Sun {get;set;}
    public bool Mon {get;set;}
    ...
    public bool Sat {get;set;} //All 7 days of the week

    public override string ToString() {
        //formatted string
        return $"{GetNumberRepresentationOfBool(Sun)},{GetNumberRepresentationOfBool(Mon)},{GetNumberRepresentationOfBool(Sat)}"
    }
}

public int GetNumberRepresentationOfBool(bool value) {
    return value ? 1 : 0
}

//printing the value
Console.WriteLine(dayDetailsObject.ToString());

Upvotes: 0

JSteward
JSteward

Reputation: 7091

You can simplify your code by converting each bool to int and joining the resulting collection.

public class DaysDetails
{
    public bool Sun { get; set; }
    public bool Mon { get; set; }
    public bool Sat { get; set; }
}

public string ConstructDays(DaysDetails d)
{
    var week = new[]
    {
        Convert.ToInt32(d.Sat),
        Convert.ToInt32(d.Sun),
        Convert.ToInt32(d.Mon),
    };
    return string.Join(",",  week);
}

Or if your looking for more than just 0/1:

public string ConstructDays(DaysDetails d)
{
    var week = new[]
    {
        d.Sat ? 0 : -1,
        d.Sun ? 1 : -1,
        d.Mon ? 2 : -1,
        //...//
    }.Where(x => x != -1);
    return string.Join(",",  week);
}

Upvotes: 5

lagripe
lagripe

Reputation: 764

Iterate through the classe properties like : 

pubic string ConstructDays(DaysDetails d)
{
 int Idx = 0;
    string days = "";
    var obj = new DaysDetails ();
    foreach (var p in obj .GetType().GetProperties())
    {   days += (bool)p.GetValue(obj ) ? (days=="" ? Idx.ToString() : ","+Idx.ToString()) : "";
        Idx++;
    }
return days
}

Upvotes: 0

Grant Winney
Grant Winney

Reputation: 66439

Define a flag enum to store your values:

[Flags]
public enum Days
{
    None = 0,
    Sun = 1,  // 0
    Mon = 2,  // 1
    Tue = 4,  // 2
    Wed = 8,  // 3
    Thu = 16, // 4
    Fri = 32, // 5
    Sat = 64  // 6
}

You can set the selected days like this:

var days = Days.None;

if (some condition)
    days |= Days.Mon;

if (some other condition)
    days |= Days.Wed;

if (yet another condition)
    days |= Days.Sat;

And generate values based on which flags are set like so:

static public string ConstructDays(Days days)
{
    return string.Join(",", Enum.GetValues(typeof(Days))
                                .Cast<Days>()
                                .Where(d => days.HasFlag(d) && d != Days.None)
                                .Select(d => Math.Log((int)d, 2)));  // 1,3,6
}

Upvotes: 0

Related Questions