Skip to main content
added 44 characters in body
Source Link
ediblecode
  • 524
  • 2
  • 10

Questionable methods

public virtual bool FileExists(string filePath) {
    bool returnValue = false;
    if (this.IsValidPath(filePath)) {
        returnValue = File.Exists(filePath);
    }

    return returnValue;
}

I'm not really sure of your intent here? What is wrong with the existing File.Exists method? IsValidPath isn't actually checking for a valid path, it's just checking !string.IsNullOrWhiteSpace(). You can remove both FileExists and IsValidPath here and replace calls to FileExists with File.Exists()

In any case, if you were to keep them, for IsValidPath this would be much more appropriate and readable:

protected virtual bool IsValidPath(string path) 
{
    return !string.IsNullOrWhiteSpace(path)
}

Styling

Usually in C# we have curly braces on new lines:

protected virtual bool IsValidPath(string path) { 

Should be:

protected virtual bool IsValidPath(string path)
{

Your concerns

Will fill in later...

Questionable methods

public virtual bool FileExists(string filePath) {
    bool returnValue = false;
    if (this.IsValidPath(filePath)) {
        returnValue = File.Exists(filePath);
    }

    return returnValue;
}

I'm not really sure of your intent here? What is wrong with the existing File.Exists method? IsValidPath isn't actually checking for a valid path, it's just checking !string.IsNullOrWhiteSpace(). You can remove both FileExists and IsValidPath here and replace calls to FileExists with File.Exists()

In any case, if you were to keep them, for IsValidPath this would be much more appropriate and readable:

protected virtual bool IsValidPath(string path) 
{
    return !string.IsNullOrWhiteSpace(path)
}

Styling

Usually in C# we have curly braces on new lines:

protected virtual bool IsValidPath(string path) { 

Should be:

protected virtual bool IsValidPath(string path)
{

Questionable methods

public virtual bool FileExists(string filePath) {
    bool returnValue = false;
    if (this.IsValidPath(filePath)) {
        returnValue = File.Exists(filePath);
    }

    return returnValue;
}

I'm not really sure of your intent here? What is wrong with the existing File.Exists method? IsValidPath isn't actually checking for a valid path, it's just checking !string.IsNullOrWhiteSpace(). You can remove both FileExists and IsValidPath here and replace calls to FileExists with File.Exists()

In any case, if you were to keep them, for IsValidPath this would be much more appropriate and readable:

protected virtual bool IsValidPath(string path) 
{
    return !string.IsNullOrWhiteSpace(path)
}

Styling

Usually in C# we have curly braces on new lines:

protected virtual bool IsValidPath(string path) { 

Should be:

protected virtual bool IsValidPath(string path)
{

Your concerns

Will fill in later...

Source Link
ediblecode
  • 524
  • 2
  • 10

Questionable methods

public virtual bool FileExists(string filePath) {
    bool returnValue = false;
    if (this.IsValidPath(filePath)) {
        returnValue = File.Exists(filePath);
    }

    return returnValue;
}

I'm not really sure of your intent here? What is wrong with the existing File.Exists method? IsValidPath isn't actually checking for a valid path, it's just checking !string.IsNullOrWhiteSpace(). You can remove both FileExists and IsValidPath here and replace calls to FileExists with File.Exists()

In any case, if you were to keep them, for IsValidPath this would be much more appropriate and readable:

protected virtual bool IsValidPath(string path) 
{
    return !string.IsNullOrWhiteSpace(path)
}

Styling

Usually in C# we have curly braces on new lines:

protected virtual bool IsValidPath(string path) { 

Should be:

protected virtual bool IsValidPath(string path)
{