Refactoring a FTPHelper Class

I've written a FTPHelper class which should help to teach me more about code structure.

I don't know what I don't know so I would really value feedback on how I can be laying out and thinking about my programming better!

```
public class FtpHelper : BaseHelper
{
public FtpHelper(string ftpHostname, string ftpUsername, string ftpPassword)
{
Hostname = ftpHostname;
Username = ftpUsername;
Password = ftpPassword;
}

private string Hostname { get; set; }
private string Username { get; set; }
private string Password { get; set; }

public void UploadFilesinFolder(string sourcePath, string destinationPath, string fileType = ".")
{
PostEvent("Destination Path: " + destinationPath, FtpEventArgs.ExceptionLevel.Debug);

if (String.IsNullOrEmpty(destinationPath)) throw new Exception("No files in destination folder or desintation folder was not specified");

foreach (var file in Directory.GetFiles(sourcePath, fileType))
{
UploadFile(file, destinationPath);
}
}

///
/// Check if a directory exists.
///
///
///
public bool DirectoryExists(string directory)
{
// todo: Check if directory var has a trailing '/', if not, add it. Otherwise false positives are thrown

if (String.IsNullOrEmpty(directory))
throw new Exception("No directory was specified to check for");

var request = (FtpWebRequest)WebRequest.Create(directory);
request.Method = WebRequestMethods.Ftp.ListDirectory;
request.Credentials = new NetworkCredential(Username, Password);

try
{
using (request.GetResponse())
{
return true;
}
}
catch (WebException)
{
return false;
}
}

public FtpStatusCode CreateDirectory(string destination)
{
var folderRequest = WebRequest.Create(destination);
folde

I would put the guard clause first - before calling the PostEvent base class method:

if (String.IsNullOrEmpty(destinationPath)) throw new Exception("No files in destination folder or desintation folder was not specified");

Now that would involve less horizontal scrolling with proper bracing:

if (String.IsNullOrEmpty(destinationPath))
{
    throw new Exception("No files in destination folder or desintation folder was not specified");
}

And now the exception type and its message become more apparent.

• Don't throw System.Exception - here an ArgumentException would be a much better fit. Always try to throw meaningful exception types.. create your own if you have to.

• The cake message is a lie. I don't see how that method can have any clue whatsoever as to how many files are in the specified destination folder. If the caller is assumed to have verified that, then this method must not make such an assumption. Misleading exception messages can make code harder to debug than it needs to be.

Same thing here - I'll just add that in no-brace code, I prefer this to the above.. but consistency in style is always a better choice:

if (String.IsNullOrEmpty(directory))
        throw new Exception("No directory was specified to check for");

The message is better though, only missing punctuation.

catch (Exception ex)
    {   
        throw new Exception("Unable to create directory " + destination + " Details:" + ex.Message);
    }

This is a very specific message, for the widest possible exception. By re-throwing (again, don't throw System.Exception) like this, you are also losing the stack trace information from the original exception - not good. A better practice would be to throw a custom exception type, and embedding the original exception as an InnerException.

StackExchange Code Review Q#73659, answer score: 6