How to combine items in List<string> to make new items efficiently

Question

I have a case where I have the name of an object, and a bunch of file names. I need to match the correct file name with the object. The file name can contain numbers and words, separated by either hyphen(-) or underscore(_). I have no control of either file name or object name. For example:

10-11-12_001_002_003_13001_13002_this_is_an_example.svg

The object name in this case is just a string, representing an number

10001

I need to return true or false if the file name is a match for the object name. The different segments of the file name can match on their own, or combined. In the example above, it should be true for the following cases (not every true case, just examples):

10001
10002
10003
11001
11002
11003
12001
12002
12003
13001
13002

And, we should return false for this case (among others):

13003

What I've come up with so far is this:

public bool IsMatch(string filename, string objectname)
{
    var namesegments = GetNameSegments(filename);
    var match = namesegments.Contains(objectname);
    return match;
}

public static List<string> GetNameSegments(string filename)
{
    var segments = filename.Split('_', '-').ToList();

    var newSegments = new List<string>();
    foreach (var segment in segments)
    {
        foreach (var segment2 in segments)
        {
            if (segment == segment2)
                continue;

            var newSegment = segment + segment2;
            newSegments.Add(newSegment);
        }
    }

    return segments.Concat(newSegments).ToList();
}

This does work so far, but is there a better way to do it, perhaps without nesting foreach loops?

Answer 1

The code you have posted looks ok, but could be improved.

• Splitting code into methods is a good thing, but in this case it would have been better if you kept it in the IsMatch() method because you could return early. In its current form you are first creating all combinations of the splitted filename and then you check if the returned list contains the objectname.

• If a parametername is a compound word you should name the parameter using camelCase casing. This means objectname and filename should be objectName and fileName.

• Omitting braces {} although they might be optional can lead to hidden and therefor hard to find bugs. I would like to encourage you to always use them.

• Calling ToList() on an array if you don't use any method of List<T> is superflous if the only intend is to iterate over the elements. But as you have a List<T> segments it would be more common to use the AddRange() method instead of Concat().

• You should always validate the parameters which are passed to a public method.

  ---

I would create a method which returns an IEnumerable<string> which iterates over an string[] if the Length of the arrayelement is equal to a desired length the method yield's the arrayelement otherwise it builds combinations of the current arrayelement with the following arrayelements.

Because this method is returning an IEnumerable<string> which is executed in a deffered and by using the Any() method which returns as soon as the passed condition is true we would also return early.

I have taken the initial method from generate-combination-using-string-array-in-c-sharp and adjusted it to our needs

private static IEnumerable<string> CreateCombinations(string[] array,  int maxLength)
{
    for (int i = 0; i < array.Length; i++)
    {
        if (array[i].Length == maxLength)
        {
            yield return array[i];
        }
        else
        {
            int length = maxLength - array[i].Length;

            foreach (string combination in array.Skip(i).Where(s => s.Length == length))
            {
                yield return string.Concat(array[i], combination);
            }
        }
    }
}

I would then extend the former IsMatch() method to take a char[] array as well. In this way you wouldn't need to touch the method again if e.g the separators should change.

This method would look like so

public static bool IsMatch(string fileName, string objectName, char[] splitChars)
{
    if (fileName == null) { throw new ArgumentNullException(nameof(fileName)); }
    if (objectName== null) { throw new ArgumentNullException(nameof(objectName)); }

    var segments = fileName.Split(splitChars);

    return CreateCombinations(segments, objectName.Length).Any(s => s == objectName);
}  

But it would look neater if we make an extension method out of the CreateCombinations method like so

public static IEnumerable<string> CreateCombinations(this string[] array, int maxLength)
{
    if (array == null) { throw new ArgumentNullException(nameof(array)); }
    for (int i = 0; i < array.Length; i++)
    {
        if (array[i].Length == maxLength)
        {
            yield return array[i];

        }
        else
        {
            int length = maxLength - array[i].Length;

            foreach (string combination in array.Skip(i).Where(s => s.Length == length))
            {
                yield return string.Concat(array[i], combination);
            }
        }
    }
}  

and the IsMatch() method would look like so

public bool IsMatch(string fileName, string objectName, char[] splitChars)
{
    if (fileName == null) { throw new ArgumentNullException(nameof(fileName)); }
    if (objectName== null) { throw new ArgumentNullException(nameof(objectName)); }

    return fileName.Split(splitChars)
                   .CreateCombinations(objectName.Length)
                   .Any(s=>s==objectName);

}