Formatting the output of a String

Question

I am new to C#. I am studying it in a module in college. We have been given an assignment which involves us having to create a simple booking application using the various components included in the toolbox in Visual Studio.

The UI has a ListBox which enables the user to select multiple names of people that will attend the event. The selected items are concatenated to a String and output in a Label when the user confirms the selection.

This is the code where I get the values from the ListBox

 protected void btnRequest_Click(object sender, EventArgs e)
{
    //Update the summary label with the details of the booking.
    n = name.Text;
    en = eventName.Text;
    r = room.SelectedItem.ToString();
    d = cal.SelectedDate.ToShortDateString();

    foreach (ListItem li in attendees.Items)
    {
        if (li.Selected)
        {
            people += li.Text + " ";
        }
    }


    confirmation.Text = r + " has been booked on " + d + " by " + n + " for " + en + ". " + people + " will be attending.";
}

Below is my entire code:

public partial class _Default : System.Web.UI.Page
{
    //Variables
    public TextBox name;
    public TextBox eventName;
    public Label confirmation;
    public DropDownList room;
    public Calendar cal;
    public Button btn;
    public ListBox attendees;

    //Booking variables - store all information relating to booking in these variables
    public String n; //name of person placing booking
    public String en; //name of event
    public String r; //room it will take place
    public List<String> att; //list of people attending
    public String d; //date it will be held on

    public String people;

    protected void Page_Load(object sender, EventArgs e)
    {
        //Get references to components
        name = txtName;
        eventName = txtEvent;
        room = droplistRooms;
        attendees = attendeelist;
        cal = Calendar1;
        btn = btnRequest;
        confirmation = lblSummary;

    }
    protected void btnRequest_Click(object sender, EventArgs e)
    {
        //Update the summary label with the details of the booking.
        n = name.Text;
        en = eventName.Text;
        r = room.SelectedItem.ToString();
        d = cal.SelectedDate.ToShortDateString();

        foreach (ListItem li in attendees.Items)
        {
            if (li.Selected)
            {
                people += li.Text + " ";
            }
        }


        confirmation.Text = r + " has been booked on " + d + " by " + n + " for " + en + ". " + people + " will be attending.";
    }

    protected void Calendar1_SelectionChanged(object sender, EventArgs e)
    {
        d = cal.SelectedDate.ToShortDateString();
    }

The output is this:

Room 2 has been booked on 08/10/2013 by Jason Manford for Comedy Gig. Jack Coldrick Bill Gates Larry Page Jimmy Wales will be attending.

However I would like to add an and to the last name of the person attending the event. How would I go about doing this. Would I have to use a List?

Many Thanks...

Answer 1

Try copying the selected items into another collection and using a simple counter:

int counter = 1; // start at 1 so that the counter is in line with the number of items that the loop has iterated over (instead of 0 which would be better for indexing into the collection)

List<ListItem> selectedItems = new List<ListItem>();

foreach (ListItem li in attendees.Items)
{
    if (li.Selected)
    {
        selectedItems.Add(li);
    }
}

foreach (ListItem li in selectedItems)
{
    counter++;

    if (selectedItems.Count > 1 && i == selectedItems.Count) // check after the counter has been incremented so that only the last item triggers it
    {
        people += " and";
    }

    people += li.Text + " ";
}

As pointed out by a few people, you should also think about using a StringBuilder, as strings are immutable in .Net, which means that they cannot be modified. Every time you append text to a string, behind the scenes a new string is being created with the new contents and the old one is being discarded. As you can imagine, if you have a lot of names in the list, this could eventually end up impacting performance. Example below:

List<ListItem> selectedItems = new List<ListItem>();

foreach (ListItem li in attendees.Items)
{
    if (li.Selected)
    {
        selectedItems.Add(li);
    }
}

StringBuilder sbPeople = new StringBuilder();
int counter = 1; // start at 1 so that the counter is in line with the number of items that the loop has iterated over (instead of 0 which would be better for indexing into the collection)

foreach (ListItem li in attendees.SelectedItems)
{
    counter++;

    if (selectedItems.Count > 1 && i == selectedItems.Count) // check after the counter has been incremented so that only the last item triggers it
    {
        sbPeople.Append(" and");
    }

    sbPeople.Append(li.Text);
    sbPeople.Append(" ");
}

Reference to the StringBuilder docs: http://msdn.microsoft.com/en-us/library/system.text.stringbuilder.aspx

Answer 2

var p = new List<string>() {"person1", "person2", "person3"};
string people;
if(p.Count == 1)
    people = p[0];
else
    people = string.Join(", ", p.Take(p.Count - 1)) + " and " + p[p.Count - 1]

To better fit your code I would write something like this (as indirectly suggested from the comments):

var p = attendees.Items.OfType<ListItem>.Where(y => y.Selected).Select(y => y.Text).ToList();
var people = "";
if(p.Count == 1)
    people = p[0];
if(p.Count > 1)
    people = string.Join(", ", p.Take(p.Count - 1)) + " and " + p[p.Count - 1]

Confirmation.Text = string.Format("{0} has been booked on {1} by {2} for {3}. {4} will be attending", r, d, n, en, people);

Answer 3

You can replace your foreach with this one

var peopleList = new List<string>();

foreach (ListItem li in attendees.Items)
{
    if (li.Selected)
       peopleList.Add(li.Text);
}

var people = string.Join(",", list.Take(list.Count - 1));
people += list.Count > 1 ? " and " + list.Last() : list.Last();

Answer 4

To keep the answer in line with your question, I'll make this easy to follow - although I'm use StringBuilder instead of appending a string - I've seen this method process larger strings much faster.

var selectedPeople = new List<string>();

foreach (ListItem li in attendees.Items)
{

    if (li.Selected)
    {
        people.Add(li.Text);
    }

}
var sb = new StringBuilder();
if (selectedPeople.Count == 0)
{
    sb.Append("");
}
else
{
    for (var i = 0; i < selectedPeople.Count; i++)
    {
        if (i > 0)
        {
            sb.Append(" ");
            if (i == selectedPeople.Count)
            {
                sb.Append("and ");
            }
        }
        sb.Append(selectedPeople[i]);
    }
}


...

confirmation.Text = r + " has been booked on " + d + " by " + n + " for " + en + ". " + sb.ToString() + " will be attending.";

I would suggest you look into the other answer which suggests using SelectedItems though, to prevent having to do the first loop.

Answer 5

var selectedPeople = list.Items.OfType<ListItem>().Where(c => c.Selected).Select(c => c.Text).ToList();
string people;
if (selectedPeople.Count == 1)
    people = selectedPeople[0];
else
    people = string.Join(", ", selectedPeople.Take(selectedPeople.Count - 1)) 
                + " and " + selectedPeople[selectedPeople.Count - 1];