Brtdku

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?

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);

}

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

}

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

}

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());