9

Variables:

private string filePath1 = null;
private string filePath2 = null;
private string filePath3 = null;
private string filePath4 = null;
private string filePath5 = null;
private string filePath6 = null;
private string filePath7 = null;
private string filePath8 = null;
private string filePath9 = null;
private string filePath10 = null;

Current If statement

if (string.IsNullOrEmpty(filePath1))
{
    errors.Add("File Not Attached");
}

if (string.IsNullOrEmpty(filePath2))
{
    errors.Add("File Not Attached");
}
....

Question:

Instead of having multiple if statements, for each variable. How can I create 1 if statement to go through all these variables?

Something like this:

if (string.IsNullOrEmpty(filePath + range(1 to 10))
{
    errors.Add("File Not Attached");
}
3
  • 7
    Use an array/list/whatever collection instead of x variables
    – Cid
    Commented Feb 24, 2020 at 13:07
  • Create a new arraylist, add all file paths to it (or initialise it with all filepaths) and the loop over the elements in the array (using for-each loop). For each element, check if nullOrEmpty and if yes add to your errors string.
    – feedy
    Commented Feb 24, 2020 at 13:07
  • 1
    TBH, I would use a Dictionary<string, string> this way if you need a specific path, you could get it by key...
    – Trevor
    Commented Feb 24, 2020 at 13:28

7 Answers 7

9

You can achieve this using Reflection. This is obviously discouraged for this scenario, as the other answers provide better solutions, just wanted to show you it's doable the way you intended it to be done (which doesn't mean it's the correct way)

public class Test
{
    private string filePath1 = null;
    private string filePath2 = null;
    private string filePath3 = null;
}

Usage:

Test obj = new Test();

//loop through the private fields of our class
foreach (var fld in obj.GetType().GetFields(BindingFlags.NonPublic | BindingFlags.Instance)
                                     .Where(x => x.Name.StartsWith("filePath"))) // filter
{
    if (string.IsNullOrEmpty(fld.GetValue(obj) as string))
    {
        errors.Add("File Not Attached in variable: " + fld.Name);
    }
}
2
  • 2
    Nice alternative solution, you could however, add a where filter to only include fields with per say, filePath to filter your results...
    – Trevor
    Commented Feb 24, 2020 at 13:33
  • 2
    @Çöđěxěŕ indeed, I just wanted to keep it simple for OP
    – Innat3
    Commented Feb 24, 2020 at 13:35
8

In nearly all cases where you're using variables with a differently numbered suffix, you should really be using a collection (array, list, ...). This is one of those cases. I'll be using a list for this answer but any collection will suffice.

private List<string> filePaths = new List<string>()
                                 {
                                     "path1",
                                     "path2",
                                     "path3",
                                     "path4"
                                 };

You can then use a loop to iterate over your list:

foreach (string path in filePaths)
{
    if(String.IsNullOrEmpty(path))
        errors.Add("File not attached");
}
4
  • Good solution, I would however probably use a Dictionary<string, string> this way if needed, you could get a path by key...
    – Trevor
    Commented Feb 24, 2020 at 13:29
  • @Çöđěxěŕ: To what end? Where does a need for a key present itself? The only identifier which OP currently uses is the number, which is covered by index-based access (i.e. myArray[5] or myList.ElementAt(5)).
    – Flater
    Commented Feb 24, 2020 at 13:33
  • As mentioned, if needed, you could get a path by key, who know's what else the OP is doing with this anyways? Currently if user needed a specific path, they would get this from the list by what, how would the user identify what string they would need, if needed?
    – Trevor
    Commented Feb 24, 2020 at 13:35
  • 1
    @Çöđěxěŕ: Adding something "just because someone might possibly use it" is a YAGNI violation. There is no need for it so it's pointless to increase the complexity because of it. Applying your logic, there are many language features I could be adding to this answer - but none of them would actually contribute to answering the asked question.
    – Flater
    Commented Feb 24, 2020 at 13:37
2

Create a new arraylist, add all file paths to it (or initialise it with all filepaths) and the loop over the elements in the array (using for-each loop). For each element, check if nullOrEmpty and if yes add to your errors string.

ArrayList arrlist = new ArrayList();
arrList.add(filePath1);
arrList.add(filePath2);
arrList.add(filePath3);
arrList.add(filePath4);
arrList.add(filePath5);
arrList.add(filePath6);
arrList.add(filePath7);
arrList.add(filePath8);
arrList.add(filePath9);
arrList.add(filePath10);

foreach (string element in arrList)
{
    if (string.IsNullOrEmpty(element)
    {
      errors.Add("File Not Attached");
    }
}

ps. You might want to print a new line after each error:

errors.Add("File Not Attached\n");
1
2
    // Create list
    List<string> filePaths = new List<string>;

    //Add path in list like 
    filePaths.add(filePath1);

    //Check for null path here

    foreach (string filepath in filePaths)
    {
        if (string.IsNullOrEmpty(filepath)
        {
          errors.Add("File Not Attached");
        }
    }
1

In order to treat all strings the same way they have to be in some collection.

using System.Linq;

...
string[] allPaths = new string[10];

// Do something with these ten paths...

if (allPaths.Any(x => string.IsNullOrEmpty(x))
    errors.Add("File Not Attached");
3
  • Note that this answer's code is not 100% the same as the posted code, as you will only ever add one error message no matter how many files are missing.
    – Flater
    Commented Feb 24, 2020 at 13:15
  • He wants to print the message for each empty path, not only once
    – feedy
    Commented Feb 24, 2020 at 13:17
  • @TeodorNarliyski I don't see that he said that explicitly; Although that had been my assumption as well. Commented Feb 24, 2020 at 13:21
0

As stated every other answers, you should use a collection.

If you really want to stick with fields names, you can use reflection, but I strongly recommend to use collections over reflection :

// using System.Reflection;
// Below code is meant to be used in a method of the class that holds the fields.
for (int i = 1; i <= 10; i++)
{
    if (string.IsNullOrEmpty(this.GetType()
                                 .GetField($"filePath{i}",
                                           BindingFlags.NonPublic | BindingFlags.Instance)?
                                 .GetValue(this))
    {
        errors.Add("File Not Attached");
    }
}
0

If you can make those variable class fields i would vote for Innat3's Answer.

Bu if this is not possible and you can't make those variables class fields then i suggest to you do like following :

class Program
    {
        static void Main(string[] args)
        {
            Dictionary<string, int> names = new Dictionary<string,int>();


            for (int i = 0; i < 10; i++)
            {
                names.Add(String.Format("name{0}", i.ToString()), i);
            }

            var xx1 = names["name1"];
            var xx2 = names["name2"];
            var xx3 = names["name3"];
        }
    }

Because in c# we can't compute dynamically variable names.

Hope this helps.

Not the answer you're looking for? Browse other questions tagged or ask your own question.