views:

107

answers:

10

Here is the loop I have so far

foreach (CheckBox chk in gpbSchedule.Controls.OfType<CheckBox>())
                {
                    if (chk.Checked)
                    {
                        //Code goes here
                    }
                }

The checkboxes all have text values of the days of the week. Monday, Tuesday ect.

I want the end result to be one string that looks like "Monday, Tuesday and Friday" depending on the whether the box is check.

The loop will change a bool so a know whether at least one checkbox is checked. This will be used in a if statement after where the produced string will be displayed so if none are checked no string will be displayed. I think this means it doesn't matter what the string looks like to start off with if that helps.

I hope I've been clear. If you need more detail please ask.

Thank you in advance.


Current code:

string days = "*";
        foreach (CheckBox chk in gpbSchedule.Controls.OfType<CheckBox>())
        {
            if (chk.Checked)
            {
                days += "#" + chk.Text;
            }
        }

        days = days.Insert(days.LastIndexOf('#'), " and ");
        days = days.Remove(days.LastIndexOf('#'), 1);
        days = days.Replace("#", ", ");
        days = days.Replace("* and ", "");
        days = days.Replace("*, ", "");

Can anyone see anything wrong with this?

A: 

Make it so the loop will keep track of all the days that you need to show the user (into a List or whatever). Then, after the loop, use string.Join to combine the first N-1 items with ", " and then a second string.Join to add the last item with " and ".

Garo Yeriazarian
+2  A: 

The easiest way I can think of is to change your foreach to a for loop. I don't have my IDE open at the moment, so I can't double check grabbing the controls but once you have a List<CheckBox> you can use (not necessary, just a little more straight-forward to my mind) you can end up with something like:

//ckBoxes is our List<CheckBox>
for(int i = 0; i < ckBoxes.Count; i++)
{
  StringBuilder listBuilder = new StringBuilder;
  if(i == ckBoxes.Count -1)
  {
    listBuilder.Append("and " + dayOfWeek)
  }
  else listBuilder.Append(dayOfWeek + ", ");
}

That's very, very rough, and needs a lot of cleaning before you use it, but it should put you on a path that works.

AllenG
You shouldn't even really need a list. You could use Linq to grab an IQueryable or a similar construct in a singular query to get the count you need.
Joel Etherton
Yeah, I mentioned that. For some reason, I just find it easier to work of the list. It's probably because every time I've done something similar, I've ended up needing a List<T> anyway, so it's easier just to put it in a list in the beginning.
AllenG
A: 

The easiest way to do this is to have two loops. The first builds a list of checked controls. Then loop over the list you just built and do the string builder commands.

List<CheckBox> checked = new List<CheckBox>();
foreach (CheckBox chk in gpbSchedule.Controls.OfType<CheckBox>())
{
    if (chk.Checked)
    {
        checked.Add(chk);
    }
}
for(int i = 0; i < checked.Count; i++)
{
    if (i == checked.Count-1))
    {
        //write for last element
    }
    else
    {
        //write for all other elements
    }
}
unholysampler
-1: 2 loops are not necessary and only increase the work done. Besides, you're not using the first loop for anything except populating a list to get the count. There are simpler ways to accomplish this.
Joel Etherton
Most of the supplied solutions are hiding the first loop by using an api that builds the list for you. I didn't write the body of if else blocks, but it should be obvious that would involve getting the object at that index of the list.
unholysampler
@unholy - You are right about everyone elses solutions. They are looping just as much as yours. I've marked yours as correct to get you some points back. Although i think my answer is much better.
Ash Burlaczenko
A: 

Something like this should work:

var days = new List<string>();
foreach (CheckBox chk in gpbSchedule.Controls.OfType<CheckBox>())
{
    if (chk.Checked)
    {
        days.Add(chk.Text);
    }
}
string daysString = "";
if (days.Count == 1)
{
    daysString = days[0];
}
else if (days.Count > 1)
{
    daysString =
        string.Join(", ", days.Take(days.Count - 1)) +
        " and " +
        days[days.Count - 1];
}
Ronald Wildenberg
+1  A: 

Try this out.

var days = gpbSchecule.Controls.OfType<CheckBox>()
                               .Where(x => x.Checked)
                               .Select(x => x.Text)
                               .ToArray();

This gets you an array containing only checked days, which you can use to determine whether 'and' is necessary, and apply simple string methods against.

From here, apply string.Join() as @Garo recommends.

kbrimington
A: 

A bit of an ugly solution but should work.

string result = "";
string nextDay = null;
foreach (CheckBox chk in gpbSchedule.Controls.OfType<CheckBox>())
{
    if (nextDay != null) {
      if (result.length() > 0) {
        result += ", " + nextDay;
      } else {
        result = nextDay;
      }
      nextDay = null;
    }
    if (chk.Checked)
    {
        //Code goes here
        nextDay = chk.text; // Your text here Monday, Tuesday, ...
    }
}

if (nextDay != null) {
  if (result.length() > 0) {
    result += " and " + nextDay;
  } else {
    result = nextDay;
  }
  nextDay = null;
}
ruibm
A: 
    // change this into a collection of your checked group boxes
    string[] threeStrings = new string[] { "Joe", "Jim", "Robert" };
    StringBuilder newString = new StringBuilder();

    // iterate over your array here - strings used to simplify example
    for (int i = 0; i < threeStrings.Length; i++)
    {
        if (i < threeStrings.Length - 1)
        {
            newString.Append(threeStrings[i]);
            newString.Append(", ");
        }
        else
        {
            newString.Append(" and ");
            newString.Append(threeStrings[i]);
        }
    }
    Console.WriteLine(newString.ToString());
Dave White
A: 

Here is another solution. I put some Init code to test it.

private List<CheckBox> _checkBoxes;

private void Test()
{
    Init();

    List<CheckBox> checkedCheckBoxes = _checkBoxes.Where(cb => cb.Checked == true).ToList();
    StringBuilder str = new StringBuilder();
    string delimiter = String.Empty;

    for (int i = 0; i < checkedCheckBoxes.Count; i++)
    {
        str.Append(delimiter);
        str.Append(checkedCheckBoxes[i].Name);

        if (i != checkedCheckBoxes.Count)
        {
            if (i == checkedCheckBoxes.Count - 2)
                delimiter = " and ";
            else
                delimiter = ", ";
        }
    }

    Console.WriteLine(str.ToString());
    Console.ReadLine();
}

private void Init()
{
    _checkBoxes = new List<CheckBox>();

    string[] days = new string[7] { "Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday" };
    Random r = new Random();

    foreach (string day in days)
    {
        CheckBox cb = new CheckBox();
        cb.Name = day;
        cb.Checked = Convert.ToBoolean(r.Next(0, 2));
        _checkBoxes.Add(cb);
    } 
}
mdm20
A: 

I voted for AllenG but you could do it this way too:

// First build a string of days separated by a coma
string days = String.Empty;
foreach (CheckBox chk in gpbSchedule.Controls.OfType<CheckBox>())
{
    if (chk.Checked)
    {
        if (!String.IsNullOrEmpty(days))
            days += ", ";
        days += chk.Text;            
    }
}

// Then replace the last coma with "and"            
int lastComaIndex = days.LastIndexOf(',');
if (lastComaIndex >= 0)
    days = days.Substring(0, lastComaIndex) + " and " + days.Substring(lastComaIndex + 2);
Danny T.
A: 

Here's my take on it:

var darr = (from checkbox in gpbSchecule.Controls.OfType<CheckBox>()
            where checkbox.Checked
            select checkbox.Text)
           .ToArray();

string days = "";
if (darr.Length > 0)
{
    days = string.Join(", ", darr.Take(darr.Length - 1));
    if (darr.Length > 1)
        days += " and ";
    days += darr[darr.Length - 1];
}
Gabe