is this code good? can be better?

Dan Raimond

Please check if this piece of code if good or could be better if yes then how and why?

I think its purpose is pretty obvious so I will skip this part. The only thing I can think of is using while instead of for. Anything else can be done better or more efficient way?

public static string GetUniqueName(List<string> existedNames, string name)
{
name = Regex.Replace(name, @" $[0-9]+$$", "");
if (existedNames.Any(n => n == name) == false)
return name;
for (int j = 0; j < 1000; j++)
{
var newName = name + " (" + j + ")";
if (existedNames.Any(n => n == newName) == false)
{
return newName;
}
}
return "";
}

2 Replies

Know the answer? Post it — somebody with the same question will find it here.

Nilesh Shah9y ago
There is always some scope of improvement in any code.

I suggest the following:

1) the parameters must be checked for null. What is I pass null as both the arguments, then your code will throw exceptions. Do the sanity check in very first line of your method.

2) Suggest to use better naming convention in for loop instead of just "j" for better readability

3) Why loop thru only 1000? If you want to make sure user always gets a unique name then append something like a random number or a portion from GUID
Nitin Sontakke9y ago
May be you should get rid of explicit checking of == false. You can just have ! instead. As in:
if(!existedNames.Any(n => n == name))
return name;
I also believe that regular expressions are expensive (no statistics at hand!). May be you can just split and use 0th element which will always be there unless you pass empty string as name.

name = name.Split(new String[] {" ("})[0];