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...

Besides an "and" you also probably want commas separating the other names. You just need to modify how you are building people. — crashmstr
– crashmstr, Commented Oct 2, 2013 at 14:19
See this: stackoverflow.com/questions/4671610/why-use-string-format — mshsayem
– mshsayem, Commented Oct 2, 2013 at 14:21

Sean Airey · Accepted Answer · 2013-10-02 15:01:24Z

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

You might want to account for the case where the list contains a single name.
Your counting scheme works, but it's not the norm. I had to look at it a few times to convince myself your i == attendees.Count wasn't a bug. If you start at 1, generally you don't increment the counter until the end of each iteration of the loop.
@Sean OP is using ASP .NET, there is no SelectedItems property in ListBox!

Tomas Jansson · Accepted Answer · 2013-10-02 17:35:03Z

3

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

edited Oct 2, 2013 at 17:35

answered Oct 2, 2013 at 14:23

Tomas Jansson

23.6k18 gold badges92 silver badges147 bronze badges

3 Comments

Corak Over a year ago

and var p = attendees.Items.Where(x => x.Selected).Select(x => x.Text).ToList(); - also, if (p.Count < 1) { people = string.empty; }

Ahmed KRAIEM Over a year ago

@Corak var p = list.Items.OfType<ListItem>().Where(c => c.Selected).Select(c => c.Text).ToList();

Corak Over a year ago

@AhmedKRAIEM - Yes, thank you. And thank everyone involved in introducing generics to .Net so I usually don't have to specify the desired type like that when working with lists/collections/whatever. ^_^;

Esteban Elverdin · Accepted Answer · 2013-10-02 14:41:35Z

2

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

edited Oct 2, 2013 at 14:41

answered Oct 2, 2013 at 14:25

Esteban Elverdin

3,5821 gold badge20 silver badges21 bronze badges

Comments

Jamie - Decodefy Ltd · Accepted Answer · 2013-10-02 14:33:21Z

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.

Ahmed KRAIEM · Accepted Answer · 2013-10-02 14:37:54Z

0

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

answered Oct 2, 2013 at 14:37

Ahmed KRAIEM

10.4k4 gold badges32 silver badges33 bronze badges

Collectives™ on Stack Overflow

Formatting the output of a String

5 Answers 5

15 Comments

3 Comments

Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

15 Comments

3 Comments

Comments

Comments

Comments

Linked

Related