patterncsharpMinor
WinSCP IDisposable Wrapper
Viewed 0 times
wrapperidisposablewinscp
Problem
I have written a simple wrapper for WinSCP in C#. I have written it to simplify SFTP connections I will needing to perform.
```
public class WinSCPConn : IDisposable
{
//private fields
private SessionOptions sessionOptions;
private TransferOptions transferOptions;
private Session session;
//public properties
public bool IsSessionOpen { get { return session.Opened; } }
public WinSCPConn(string hostname, string username, string password)
{
sessionOptions = new SessionOptions();
sessionOptions.Protocol = Protocol.Sftp;
sessionOptions.HostName = hostname;
sessionOptions.PortNumber = 22;
sessionOptions.Password = password;
sessionOptions.UserName = username;
sessionOptions.Timeout = new TimeSpan(0, 3, 0);
sessionOptions.GiveUpSecurityAndAcceptAnySshHostKey = true;
transferOptions = new TransferOptions();
transferOptions.TransferMode = TransferMode.Binary;
try
{
session = new Session();
session.ExecutablePath = Properties.Settings.Default.WinSCPPath;
}
catch (SessionLocalException)
{
throw;
}
}
public void Open()
{
try
{
if (session.Opened == false)
{
session.Open(sessionOptions);
}
}
catch (SessionRemoteException)
{
throw;
}
}
public TransferOperationResult SendFile(string SourceFile, string DestFile)
{
TransferOperationResult result;
try
{
result = session.PutFiles(SourceFile, DestFile, false, transferOptions);
return result;
}
catch (SessionRemoteException)
```
public class WinSCPConn : IDisposable
{
//private fields
private SessionOptions sessionOptions;
private TransferOptions transferOptions;
private Session session;
//public properties
public bool IsSessionOpen { get { return session.Opened; } }
public WinSCPConn(string hostname, string username, string password)
{
sessionOptions = new SessionOptions();
sessionOptions.Protocol = Protocol.Sftp;
sessionOptions.HostName = hostname;
sessionOptions.PortNumber = 22;
sessionOptions.Password = password;
sessionOptions.UserName = username;
sessionOptions.Timeout = new TimeSpan(0, 3, 0);
sessionOptions.GiveUpSecurityAndAcceptAnySshHostKey = true;
transferOptions = new TransferOptions();
transferOptions.TransferMode = TransferMode.Binary;
try
{
session = new Session();
session.ExecutablePath = Properties.Settings.Default.WinSCPPath;
}
catch (SessionLocalException)
{
throw;
}
}
public void Open()
{
try
{
if (session.Opened == false)
{
session.Open(sessionOptions);
}
}
catch (SessionRemoteException)
{
throw;
}
}
public TransferOperationResult SendFile(string SourceFile, string DestFile)
{
TransferOperationResult result;
try
{
result = session.PutFiles(SourceFile, DestFile, false, transferOptions);
return result;
}
catch (SessionRemoteException)
Solution
-
a condition to be met like
-
-
you should declare the variable as near as possible to its usage and instead of catching .. rethrowing the exception, let it bubble up the tree. So it can be simplified to
-
if comments don't add any value, like explaining why something is done (what is done should be explained by the code itself), they should be removed like
a condition to be met like
if (someBoolean == true) can be simplified to if (someBoolean). For reverse check use the Not operator ! like if (!someBoolean). -
Dipsose() should not only call Close() but also get rid of session, sessionOptions and transferOptions by calling Dispose() if they implement IDisposible or at least setting them to null. -
SendFile() you should declare the variable as near as possible to its usage and instead of catching .. rethrowing the exception, let it bubble up the tree. So it can be simplified to
public TransferOperationResult SendFile(string SourceFile, string DestFile)
{
return session.PutFiles(SourceFile, DestFile, false, transferOptions);
}-
if comments don't add any value, like explaining why something is done (what is done should be explained by the code itself), they should be removed like
//public propertiesCode Snippets
public TransferOperationResult SendFile(string SourceFile, string DestFile)
{
return session.PutFiles(SourceFile, DestFile, false, transferOptions);
}//public propertiesContext
StackExchange Code Review Q#75131, answer score: 4
Revisions (0)
No revisions yet.