patterncsharpMinor
Immutable File Upload Server Configuration Class (FTP or Network Drive)
Viewed 0 times
fileftpuploadimmutableserverconfigurationclassnetworkdrive
Problem
The application I'm working on needs to occasionally take pictures, and then send them to either an ftp server or a network drive (user configurable). To try to abstract away the differences between a ftp server and a network drive I made this class.
The idea is, the code that takes a picture with the camera and saves the image just needs to call either a WriteFTP or WriteLocal method with the URL from this class to get the image where it needs to go.
Since the class doing the picture taking and image writing is on a separate thread, I decided to make the server configuration immutable.
I feel like there's a better way to implement this, possibly using inheritance, because setting all of these configuration options in the constructor is rather cumbersome.
However I'm not sure if it would have been better to have four inherited classes, because really there's 4 kinds of saving options:
-
Local or network drive with a folder that can be specified at the time the file is saved (e.g., you could make a folder for each hour)
-
Local or network drive with a specified daytime and night time folder (e.g., "shift1", "shift2")
-
FTP folder that can be specified at the time the file is saved
-
FTP folder with a specified daytime and night time folder
```
// I considered having this be the base class, and having two classes inherit from it, e.g.,
// FTPDirectory : SaveDirectory, and
// LocalOrNetworkDirectory : SaveDirectory;
//
// I am still not sure if that would have been better
internal class SaveDirectory
{
private SaveDirectory(string description, LocationType type, bool usedynamicfolders, string dynamic_rootfolder, string custom_daytimefolder, string custom_nightfolder,
string ftp_username, string ftp_password, Int16 ftp_port, string ftp_hostname)
{
this.Description = description;
this.Type = type;
this.UseDynamicFolders = usedynamicfolders;
this.Dynamic_RootFolder = dynamic_rootfolder;
this.Custom_Day
The idea is, the code that takes a picture with the camera and saves the image just needs to call either a WriteFTP or WriteLocal method with the URL from this class to get the image where it needs to go.
Since the class doing the picture taking and image writing is on a separate thread, I decided to make the server configuration immutable.
I feel like there's a better way to implement this, possibly using inheritance, because setting all of these configuration options in the constructor is rather cumbersome.
However I'm not sure if it would have been better to have four inherited classes, because really there's 4 kinds of saving options:
-
Local or network drive with a folder that can be specified at the time the file is saved (e.g., you could make a folder for each hour)
-
Local or network drive with a specified daytime and night time folder (e.g., "shift1", "shift2")
-
FTP folder that can be specified at the time the file is saved
-
FTP folder with a specified daytime and night time folder
```
// I considered having this be the base class, and having two classes inherit from it, e.g.,
// FTPDirectory : SaveDirectory, and
// LocalOrNetworkDirectory : SaveDirectory;
//
// I am still not sure if that would have been better
internal class SaveDirectory
{
private SaveDirectory(string description, LocationType type, bool usedynamicfolders, string dynamic_rootfolder, string custom_daytimefolder, string custom_nightfolder,
string ftp_username, string ftp_password, Int16 ftp_port, string ftp_hostname)
{
this.Description = description;
this.Type = type;
this.UseDynamicFolders = usedynamicfolders;
this.Dynamic_RootFolder = dynamic_rootfolder;
this.Custom_Day
Solution
The huge constructore is the result that the
I suggest the following structure:
Define a storage interface:
Create two storages that implement the interface:
Extract the enum type from the class to not be nested:
Create a storage factory that decides based on the parameter which storage to create and knows how to initialize each one. The settings come from a configuration I guess?
Now you can easily add more storages anytime and you can test the ones you already have without affecting the others. Their constructors require less parameters and are easier to use.
SaveDirectory class is doing too much. It has to know everything about the ftp server as well as about the network drive. You need to separate them.I suggest the following structure:
Define a storage interface:
interface IImageStorage
{
bool SaveImage(Image image);
// if there are any other common methods or properties
// you can define them here too
}Create two storages that implement the interface:
class FtpImageStorage : IImageStorage
{
// the constructor and
// all ftp properties go into this class
public bool SaveImage(Image image) { return false; }
}
class NetworkDriveImageStorage : IImageStorage
{
// the constructor and
// all network drive properties go into this class
public bool SaveImage(Image image) { return false; }
}Extract the enum type from the class to not be nested:
internal enum LocationType
{
// no reason for this to be 1 other than 0 being the default value for ints
UNC_Path = 1,
FTP
};Create a storage factory that decides based on the parameter which storage to create and knows how to initialize each one. The settings come from a configuration I guess?
class ImageStorageFactory
{
// register the storages here
private static readonly Dictionary> _createStorageFuncs =
new Dictionary>
{
[LocationType.UNC_Path] = CreateNetworkDriveImageStorage,
[LocationType.FTP] = CreateFtpImageStorage,
};
// you can also name the method GetImageStorage and cache them if necessary
public static IImageStorage CreateImageStorage(LocationType locationType)
{
return _createStorageFuncs[locationType]();
}
private static IImageStorage CreateFtpImageStorage()
{
// ftp image storage initialization
return new FtpImageStorage();
}
private static IImageStorage CreateNetworkDriveImageStorage()
{
// network drive storage initialization
return new NetworkDriveImageStorage();
}
}Now you can easily add more storages anytime and you can test the ones you already have without affecting the others. Their constructors require less parameters and are easier to use.
Code Snippets
interface IImageStorage
{
bool SaveImage(Image image);
// if there are any other common methods or properties
// you can define them here too
}class FtpImageStorage : IImageStorage
{
// the constructor and
// all ftp properties go into this class
public bool SaveImage(Image image) { return false; }
}
class NetworkDriveImageStorage : IImageStorage
{
// the constructor and
// all network drive properties go into this class
public bool SaveImage(Image image) { return false; }
}internal enum LocationType
{
// no reason for this to be 1 other than 0 being the default value for ints
UNC_Path = 1,
FTP
};class ImageStorageFactory
{
// register the storages here
private static readonly Dictionary<LocationType, Func<IImageStorage>> _createStorageFuncs =
new Dictionary<UserQuery.LocationType, Func<IImageStorage>>
{
[LocationType.UNC_Path] = CreateNetworkDriveImageStorage,
[LocationType.FTP] = CreateFtpImageStorage,
};
// you can also name the method GetImageStorage and cache them if necessary
public static IImageStorage CreateImageStorage(LocationType locationType)
{
return _createStorageFuncs[locationType]();
}
private static IImageStorage CreateFtpImageStorage()
{
// ftp image storage initialization
return new FtpImageStorage();
}
private static IImageStorage CreateNetworkDriveImageStorage()
{
// network drive storage initialization
return new NetworkDriveImageStorage();
}
}Context
StackExchange Code Review Q#133954, answer score: 4
Revisions (0)
No revisions yet.