0

I've three three textBox, textBoxLectura1, textBoxLectura2 and textBoxLectura3. When someone clicks on the button button3 I want just to print the entries the user has written.

I made this code and works perfect. I would like to know if there's a more elegant/efficent way to do it without using so many if statements. I would like to maintain the arrayList structure.

 private void button3_Click(object sender, EventArgs e) { ArrayList myarray2 = new ArrayList(); if (string.IsNullOrWhiteSpace(textBoxLectura1.Text) == false) { myarray2.Add(textBoxLectura1.Text); } if (string.IsNullOrWhiteSpace(textBoxLectura2.Text) == false) { myarray2.Add(textBoxLectura2.Text); } if (string.IsNullOrWhiteSpace(textBoxLectura3.Text) == false) { myarray2.Add(textBoxLectura3.Text); } if (myarray2.Count > 0) { foreach (string values in myarray2) { Console.WriteLine(values ); } } else { MessageBox.Show("no entrys"); } }
Improve this question
5
  • 1
    Do if(!string.IsNullOrWhiteSpace(textBoxLectura1.Text)) instead of if (string.IsNullOrWhiteSpace(textBoxLectura1.Text) == false). Commented Apr 23, 2015 at 14:10
  • 1
    First thing - dont use == false ... use ! operator like this: (!string.IsNullOrWhiteSpace(textBoxLectura3.Text) Commented Apr 23, 2015 at 14:10
  • Why the downvote if im just asking for an improvement for a code I did? Commented Apr 23, 2015 at 14:12
  • 2
    @Borja: Code review is often off-topic. There is a code review stack exchange site that would usually be a better home. (note: I wasn't the downvoter). Commented Apr 23, 2015 at 14:18
  • Thanks for the comment @MattBurland . I didn't knew there was a code review site. I will use that next time. Commented Apr 23, 2015 at 14:20

6 Answers 6

Reset to default
8

The simplest way might be to do something like:

var strings = new List<string>() { textBoxLectura1.Text, textBoxLectura2.Text, textBoxLectura3.Text }; var output = string.Join('\n',strings.Where(s => !string.IsNullOrEmpty(s)); if (!string.IsNullOrEmpty(output)) { Console.Writeline(output); } else { MessageBox.Show("no entrys"); }

A couple of points here. Don't use ArrayList. A typed List<T> is better in pretty much every way. ArrayList is a hold over from C# when it didn't have generics. The only reason to ever use ArrayList is if you are dealing with some legacy (pre 2.0) code that needs it. A List<object> is functional equivalent to an ArrayList, but since a lot of the time you are dealing with homogeneous collections it's much easier to use a List<T>, where T is whatever type you are populating your collection with (in this case string), and not have to deal with casting every time you try and get something from your collection.

What we do in the code here is we create a List<string> and immediately initialize it with the values of all our textboxes .Text property. It doesn't matter if they are null or empty at the point.

Then we use the really helpful string.Join method. This takes a collection of strings and glues them together with a separator. In this case, we use the newline character (\n). But, since we don't want to include nulls or empty strings (String.Join would otherwise insert extra newline characters), we use a simple LINQ .Where statement to select only those strings that aren't null or empty.

Improve this answer
Sign up to request clarification or add additional context in comments.

11 Comments

This looks great, but as I said on the post, I would like to work with arrayList.
@Borja Why ArrayList? It shouldn't be used anymore. List<T> supersedes it and it is type safe.
There's no valid reason to use ArrayList since .net 1.1.
As I'm new in c# (started two weeks ago) and I did a lot of use of ArrayList in Java which are the data structures that I feel more confortable with. But I will try the List<String>. Thanks.
@Borja: The only reason to ever use ArrayList is if you are dealing with some legacy (pre 2.0) code that needs it. A List<object> is functional equivalent to an ArrayList, but since a lot of the time you are dealing with homogeneous collections it's much easier to use a List<T>, where T is whatever type you are populating your collection with, and not have to deal with casting every time you try and get something from your collection
|
4

Here's my take on it.

var notEmpty = new[] { textBoxLectura1.Text, textBoxLectura2.Text, textBoxLectura3.Text} .Where(s => !String.IsNullOrEmpty(s) .ToArray(); if (!notEmpty.Any()) { MessageBox.Show("No Entries"); return; } Console.WriteLine(string.Join(Environment.NewLine, notEmpty));

I also agree with other posters, ArrayList vs an IEnumerable<T> implementing container is a no-brainer, you should be using a strongly-typed collection type.

Improve this answer

Comments

1

You can use a function to Check & Add:.

private void button3_Click(object sender, EventArgs e) { ArrayList myarray2 = new ArrayList(); ChecknAdd(textBoxLectura1, myarray2); ChecknAdd(textBoxLectura2, myarray2); ChecknAdd(textBoxLectura3, myarray2); if (myarray2.Count > 0) { foreach (string values in myarray2) { Console.WriteLine(values); } } else { MessageBox.Show("no entrys"); } } void ChecknAdd(TextBox txt, System.Collections.ArrayList Ary) { if (string.IsNullOrWhiteSpace(txt.Text) == false) { Ary.Add(txt.Text); } }
Improve this answer

Comments

1

create a procedure like this:

private void addOnArray(ArrayList array, String text) { if (string.IsNullOrWhiteSpace(text) == false) { array.Add(text); } }

And in the caller:

private void button3_Click(object sender, EventArgs e) { ArrayList myarray2 = new ArrayList(); addOnArray(myarray2, textBoxLectura1.Text); addOnArray(myarray2, textBoxLectura2.Text); addOnArray(myarray2, textBoxLectura3.Text); if (myarray2.Count > 0) { foreach (string values in myarray2) { Console.WriteLine(values ); } } else { MessageBox.Show("no entrys"); } }
Improve this answer

Comments

0

You can shorten your if(s) to ternary operators... When you add to a List, it'll return the index it was added to, otherwise just assign a -1 to the receiving variable

Then use the same ternary logic to know if you want to print the contents of your ArrayList, or just print "No Entries"

 ArrayList myarray2 = new ArrayList(); int index = !string.IsNullOrWhiteSpace(textBoxLectura1.Text) ? myarray2.Add(textBoxLectura1.Text) : -1; index = !string.IsNullOrWhiteSpace(textBoxLectura2.Text) ? myarray2.Add(textBoxLectura2.Text) : -1; index = !string.IsNullOrWhiteSpace(textBoxLectura3.Text) ? myarray2.Add(textBoxLectura3.Text) : -1; Console.WriteLine(myarray2.Count > 0 ? string.Join("\n", myarray2.ToArray()) : "No Entries");
Improve this answer

Comments

0

I really appreciate the approaches in other answers and the way couple of them have explained why to use Generics vs ArrayLists. This is how I perform it (learnt it thanks to SO):

IEnumerable<TextBox> textBoxCollection = this.Controls.OfType<TextBox>(); foreach (TextBox box in textBoxCollection) { if (!string.IsNullOrWhiteSpace(box.Text)) { MessageBox.Show(box.Text); } }
Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.