Pet peeves about directory and file referencing
Since starting at Microsoft I have had to review a lot of code - both in code reviews and when debugging others' tests. Like most other people I have my pet peeves and nitty gritty details. For instance, instead of writing
if
(somevariable == null)
I often recommend that people use
if
(null == somevariable)
How does this help you? Well, if you accidentally type
if (somevariable = null)
You will assign null to somevariable - probably not what you wanted to do. Yes, I do know that the C# compiler will warn you about this but languages such as C and C++ will not be so forgiving. However, if this piece of code is reversed so the null is on the left hand side the compiler will always generate an error because null cannot be assigned a value.
That, however, is only a small detail and is a matter of coding style. My biggest pet peeve is when I see code that looks like the following
string
fullFilePath = path + "\\" + fileName;
Why is this a problem? What if the variable path already has a backslash? If that is the case the path will be invalid and an error will occur at runtime. The .NET runtime makes quick work of situations like this
string
fullFilePath = Path.Combine(path, fileName);
The Path class is available is System.IO and contains many other useful methods - all of which are documented in MSDN. One of my other favorite methods is Path.GetTempFileName() - which returns a path in the user's temp directory - the only place where you are guaranteed to always have write access. Getting back to the code above isn't it much cleaner? In this case Path.Combine takes care of all of the possibilities for you - you no longer have to worry about the backslash in the file name. One time I saw code similar to the following
bool
result = SomeMethod(data, someDir + "\\");
Now this is just plain ugly. Here we are calling a method that depends on the directory name ending with a backslash. This is just asking for errors. This can be fixed by rewriting SomeMethod to use Path.Combine to handle the backslash.
Another problem with hard coding the backslash is several localized OS's do not use the backslash - for instance the Japanese OS uses the Yen sign. Of course the .NET Path class will handle this for you - it even has a property that returns the separator character for the current system if you are interested.
Comments
- Anonymous
February 03, 2005
Actually "string fullFilePath = path + "" + fileName;" is perfectly fine, even IF path contains a trailing backslash.
This is because the Win32 path canonicalization code converts / to and collapses adjacent characters.
On the other hand, there ARE two things that are horribly wrong with "string fullFilePath = path + "" + fileName;".
The first is that it uses std::string, which has some huge issues. The second is that it constructs two temporary string objects to deal with the interim expressions. That means you take three trips through the heap allocator for that.
You'd be better off with:
string fullFilePath(path);
fullFilePath += "";
fullFilePath += fileName;
That only generates one temporary std::string object (and potentially none if std::string defines operator+=(const char *) - Anonymous
February 03, 2005
This is one of the things I love about .NET. It seems as if there are classes for many of the obscure things that you need to do. Path.Combine is a godsend. It would take a lot of code to roll it correctly on your own. - Anonymous
February 03, 2005
Actually, on a Japanese system, the Yen symbol has the value 0x5C, which is the same as the backslash character on a US-ASCII system. So if you hard-code @"" in your application, it'll still work on a Japanese system (because @"" has the same binary value as the Yen symbol).
See, for example, http://weblogs.asp.net/michkap/archive/2004/12/14/284838.aspx (the comments are probably most useful)
Of course, it won't work on a Unix/BSD system where the path separator is "/". So your point still stands. - Anonymous
February 03, 2005
Larry, from context I think we're coding in C# here, so we're actually dealing with a System.String. C# optimizes multiple uses of + in the same statement to a call to String.Concat, which performs one allocation from the GC heap.
If you wanted to perform the same operation in unmanaged C++, you'd call PathCombine from shlwapi.dll. Despite the similarity of names, System.IO.Path.Combine does not call PathCombine - presumably the marshalling and interop overhead outweighs the cost of reimplementation.