Problem
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?
Solution
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 meansobjectname
andfilename
should beobjectName
andfileName
. -
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 ofList<T>
is superflous if the only intend is to iterate over the elements. But as you have aList<T> segments
it would be more common to use theAddRange()
method instead ofConcat()
. -
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);
}