-1
if (text != "" || text != null)
                    myList[i] = text;

myList is string array But in the end of the loop i keep see in myList items that are "" or null.

WindowSnap[] windowsarray = WindowSnap.GetAllWindows(true, true).ToArray();
            this.listBoxSnap.BeginUpdate();
            this.listBoxSnap.Items.Clear();           
            this.listBoxSnap.Items.AddRange(windowsarray);
            string[] myList = new string[listBoxSnap.Items.Count];
            for (int i = listBoxSnap.Items.Count - 1; i >= 0; i--)
            {
                string tt = listBoxSnap.Items[i].ToString();

                int first = tt.IndexOf("Text: ");
                int second = tt.IndexOf(",", first + 6);
                string text = tt.Substring(first + 6, second - first - 6);
                if (text != "" || text != null)
                    myList[i] = text;
            }

I tried inside the loop and after the loop the line:

myList.Where(x => !string.IsNullOrEmpty(x)).ToArray();

But still there are items with ""

Daniel Lipman
  • 41
  • 1
  • 8
  • 4
    If you don't want to put an empty string or null into the array, use &&, not ||. Better yet, use !string.IsNullOrEmpty(value). – Anthony Pegram May 17 '16 at 16:33
  • It would also be better to reverse it and use the `.Equals()` method. `if (!text.Equals(null) && !text.Equals(string.empty))` – Draken May 17 '16 at 16:34
  • @Draken Why would `Equals` be better? Both end up doing the exact same thing. – juharr May 17 '16 at 16:38
  • You might want to use a `List` instead. When you initialize the `string[]` it going to be populated with nulls. – juharr May 17 '16 at 16:39
  • @juharr [See here for why](http://stackoverflow.com/a/814881/833070) – Draken May 17 '16 at 16:41
  • @Draken But `text` is a `string` reference as is `""`, so you don't have the issue of something like `text == ((object)"")`. And the `null` check is going to be a reference check anyway. And hopefully you don't expect someone to refactor the code to change `string` variables to `object`. – juharr May 17 '16 at 16:45
  • You don't assign `myList[i] = text` if text is empty *but* you change `i` in the loop. Use `List myList` and `myList.Add(text);` instead. – Alex Kudryashev May 17 '16 at 16:57
  • I forgot to mention that after parsing the text of the items inside the loop i want to update the items text in the listBox already. Not to add new items of myList so maybe it's better to work directly with the listBox items somehow. What i want is to parse only the items(that already exist in the listBox) text in the listBox. Update i mean change/replace but only the text !!! with the text of each item in the listBox. – Daniel Lipman May 17 '16 at 17:03
  • Related to [Why getting all the items in listBox null when trying to remove part of each item text?](http://stackoverflow.com/q/37279631/719186) – LarsTech May 17 '16 at 19:06

3 Answers3

4

Instead of doing

if(text != "" || text != null)

Do

if(!String.IsNullOrEmpty(text))

The OR in your condition is wrong. It should be AND.

Also note that it would be better if you use List<string> instead of String[]. Arrays are fixed size, so you'll still get null values in your output for the items that are skipped by the above condition. A List on the other hand can be grown one by one when needed. So your code would look like:

WindowSnap[] windowsarray = WindowSnap.GetAllWindows(true, true).ToArray();
this.listBoxSnap.BeginUpdate();
this.listBoxSnap.Items.Clear();           
this.listBoxSnap.Items.AddRange(windowsarray);

List<string> myList = new List<string>();
for (int i = listBoxSnap.Items.Count - 1; i >= 0; i--)
{
   string tt = listBoxSnap.Items[i].ToString();
   int first = tt.IndexOf("Text: ");
   int second = tt.IndexOf(",", first + 6);
   string text = tt.Substring(first + 6, second - first - 6);
   if (!String.IsNullOrEmpty(text))
       myList.Add(text);
}

If you are a fan of LINQ, you may do it like this:

string[] mylist = (from tt in listBoxSnap.Items.Cast<object>().Select(x => x.ToString())
           let first = tt.IndexOf("Text: ")
           let second = tt.IndexOf(",", first + 6)
           let text = tt.Substring(first + 6, second - first - 6)
           where !String.IsNullOrEmpty(text)
           select text).Reverse().ToArray();

Note that I'm Casting Items collection to object. If you know the actual type of the items in listBoxSnap, you may want to cast it to that type.

dotNET
  • 33,414
  • 24
  • 162
  • 251
  • He doesn't want to remove things from the original array as per OP. – dotNET May 17 '16 at 16:37
  • Ah you mean the output array, not the original array. I have updated my answer. – dotNET May 17 '16 at 16:44
  • I forgot to mention that after making/creating the myList array i don't want to add this array as items to the listBox i want to change the existing items already in the listBox text only. Maybe i should now use the myList array variable at all and working directly with the windowsarray or better directly with the listBox items already added. What i want is to remove some of the text of each item. i did it with the indexof and the substring but how do i update now the items text in the listBox ? – Daniel Lipman May 17 '16 at 17:01
  • @DanielLipman: What is `WindowSnap`? Note that you're using `ToString()` against its items to populate you `ListBox`, which returns a **value that has no link with the original `WindowSnap` object**. If `WindowSnap` has a public property that returns its name, a much better way is to use binding. You can then get access to the original WindowSnap object from within ListBox item and change it accordingly. – dotNET May 17 '16 at 18:01
2

First of all this line:

string[] myList = new string[listBoxSnap.Items.Count];

creates a string array with as many elements as your listBoxSnap.Items. All these elements are initialized to null. So the null elements are there from the beginning.

Second, in your loop you set the elements if (text != "" || text != null). This is true for all texts, because || means OR. So either it is null (so it's not "") or it is not null (so it's true too).

So you better make this array a List:

List<string> myList = new List<string>();

and in your loop you do:

if (!string.IsNullOrEmpty(text)) myList.Add(text);
René Vogt
  • 43,056
  • 14
  • 77
  • 99
0
if (text != ""  ||  text != null)
    myList[i] = text;

This reminds me of a moral tale about programming that I heard during my college years. The story (supposedly true) goes that a young programmer was working on a financial program which had a rule that applied an exemption for people under 18 and over 65. So to implement this rule, the programmer wrote code like this:

if (age < 18  and  age > 65)
    exempt = true;

Needless to say, his co-workers eventually noticed the flaw in his code and tried to explain it to him, but he was adamant that he was implementing the rule exactly as it was stated in the requirements: exemptions are for customers under 18 and customers over 65. He did not see why he needed to use or in his conditional test.

Moral

For (the hopefully few of) those of you who do not see what the problem is, the test should be this:

if (text != ""  &&  text != null)
    myList[i] = text;

If you still don't see it, ask yourself what happens when text is null.

David R Tribble
  • 11,918
  • 5
  • 42
  • 52