public bool isLocked(string s)
{
if (!File.Exists(s)) return false;
try { FileStream f = File.Open(s, FileMode.Open, FileAccess.ReadWrite); f.Close(); }
catch { return true; }
return false;
}Objections and Comments
The code abve has in my eyes several problems:
- Method name starts with lower case
- The name of the input parameter is meaningless
- Logical code blocks are not separated by a new line
- You need some time to understand what this 4 lines of code do
- Multiple instructions on the same line
- The "return false;" statement should be inside the try block because it is logically associated to it
- Missing Method documentation
Proposed Refactoring
/// <summary>
/// Checks if a file is in use or not.
/// </summary>
/// <param name="filePath">The path to the file that should be checked.</param>
/// <returns>True if the file is locked; false otherwise</returns>
public bool IsLocked(string filePath)
{
if (!File.Exists(filePath))
{
return false;
}
try
{
FileStream fileStream = File.Open(filePath, FileMode.Open, FileAccess.ReadWrite);
fileStream.Close();
return false;
}
catch
{
return true;
}
}