FileDialogSettings - Additional Constructors
RFBomb opened this issue · 9 comments
Is your feature request related to a problem? Please describe.
FileDialogSettings is currently abstract, so whenever you need to create ether Open/Save dialog, you have to specfiy everything.
Describe the solution you'd like
Remove the 'Abstract' from the FileDialogSettings, and create new constructors for the derived classes that accepts the base class.
Easiest way to do this would be to create a protected constructor within FileDialogSettings that accepts itself as a parameter. Then its just assigning the values to itself.
This also allows for taking the result of a OpenFileDialog and creating the SaveFileDialog from it, since they both inherit from the base class.
Hi @RFBomb!
Could you please provide an example of the code you currently are required to write, and then another example of the code you would like to write given we change this library according to your suggestions? Thanks!
Here is my current 'factory' / extension method so I could have a shortcut to creating the diag settings
/// <summary>
/// Create the Dialog Settings
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="title"><inheritdoc cref="FileDialogSettings.Title" path="*"/></param>
/// <param name="fileName"><inheritdoc cref="FileDialogSettings.FileName" path="*"/></param>
/// <param name="filter"><inheritdoc cref="FileDialogSettings.Filter" path="*"/></param>
/// <param name="defaultExt"><inheritdoc cref="FileDialogSettings.DefaultExt" path="*"/></param>
/// <param name="initialDirectory"><inheritdoc cref="FileDialogSettings.InitialDirectory" path="*"/></param>
/// <param name="customPlaces"><inheritdoc cref="FileDialogSettings.CustomPlaces" path="*"/></param>
/// <returns>new Dialog Settings of type <typeparamref name="T"/></returns>
private static T CreateFileDialogSettings<T>(string title, string fileName = "", string filter = DefaultExt, string defaultExt = "", string initialDirectory = null, IList<Microsoft.Win32.FileDialogCustomPlace> customPlaces = null) where T: FileDialogSettings, new()
{
var diag = new T()
{
AddExtension = false,
DereferenceLinks = true, //Convert shortcuts to actual file paths
CheckPathExists = true,
DefaultExt = defaultExt,
Filter = filter,
FileName = fileName,
InitialDirectory = initialDirectory ?? Path.GetDirectoryName(fileName),
Title = title,
ValidateNames = true,
};
if (customPlaces != null) diag.CustomPlaces = customPlaces;
return diag;
}
#region < Save File Browser >
/// <summary>
/// Get a new FolderBrowserDialogSettings object
/// </summary>
/// <returns>new FolderBrowserDialog settings</returns>
public static SaveFileDialogSettings GetSaveFileDialogSettings(this MvvmDialogs.IDialogService service) { return CreateFileDialogSettings<SaveFileDialogSettings>("Save File"); }
/// <param name="service">Not Required - Only for shortcut purposes</param>
/// <param name="overWritePrompt"> <inheritdoc cref="SaveFileDialogSettings.OverwritePrompt" path="*"/></param>
/// <param name="filters">File Filter objects to define the desired filters</param>
/// <returns></returns>
/// <inheritdoc cref="CreateFileDialogSettings{T}"/>
/// <param name="fileName"/>
/// <param name="initialDirectory"/>
/// <param name="title"/>
/// <param name="defaultExt"/>
public static SaveFileDialogSettings GetSaveFileDialogSettings(this MvvmDialogs.IDialogService service, string fileName, bool overWritePrompt = true, string initialDirectory = "", string title = "Open File", string defaultExt = "", params FileFilter[] filters)
{
var diag = CreateFileDialogSettings<SaveFileDialogSettings>("Open", fileName, filters?.GetMvvmDialogFilter(), defaultExt, initialDirectory, null);
diag.CreatePrompt = false;
diag.OverwritePrompt = overWritePrompt;
diag.CheckFileExists = true;
return diag;
}
As can be seen, I have a static method to create the dialog settings but I use a private generic to apply the desired defaults. That way those defaults can be applied to either the open or save dialogs upon instantation.
This method is fine for my purposes, as the open/close operations typically are very detached for my application.
I was just thinking that being able to instantiate using the base class could be convenient should an application do a Open-Modify-Save operation. The open dialog could be instantiated, than that object gets passed into the save constructor for the save constructor to adopt those settings.
I'm on mobile right now so can't mock it up, but can maybe tomorrow
and here is my suggestion I whipped up this morning that shows some additional constructors for (what I assume) would be the primary settings people modify.
public class OpenFileDialogSettings : FileDialogSettings
{
/// <summary> Create a new OpenFileDialogSettings with the default values </summary>
public OpenFileDialogSettings2() : base() { }
/// <summary>Create a new OpenFileDialogSettings using the settings from a previous dialogue </summary>
/// <inheritdoc cref="FileDialogSettings.FileDialogSettings(FileDialogSettings, string, string, string)"/>
public OpenFileDialogSettings(string title = "Open File", string filter = "All Files|*.*", string initialDirectory = null, string fileName = "") : base(title, filter, initialDirectory, fileName) { }
/// <summary>Create a new OpenFileDialogSettings using the settings from a previous dialogue </summary>
/// <inheritdoc cref="FileDialogSettings.FileDialogSettings(FileDialogSettings, string, string, string)"/>
public OpenFileDialogSettings2(FileDialogSettings settings, string title = "Open File", string fileName = null, string initialDirectory = null) : base(settings, title, fileName, initialDirectory) { }
}
public class SaveFileDialogSettings : FileDialogSettings
{
/// <summary> Create a new SaveFileDialogSettings with the default values </summary>
public SaveFileDialogSettings() : base() { }
/// <summary>Create a new SaveFileDialogue using the settings from a previous dialogue </summary>
/// <inheritdoc cref="FileDialogSettings.FileDialogSettings(FileDialogSettings, string, string, string)"/>
public SaveFileDialogSettings(string title = "Save File", string filter = "All Files|*.*", string initialDirectory = null, string fileName = "") : base(title, filter, initialDirectory, fileName) { }
/// <summary>Create a new SaveFileDialogue using the settings from a previous dialogue </summary>
/// <inheritdoc cref="FileDialogSettings.FileDialogSettings(FileDialogSettings, string, string, string)"/>
public SaveFileDialogSettings(FileDialogSettings settings, string title = "Save File", string fileName = null, string initialDirectory = null) : base(settings, title, fileName, initialDirectory) { }
}
public class FileDialogSettings
{
/// <summary> Create a new FileDialogSettings with the default values </summary>
public FileDialogSettings() : base() { }
/// <summary>
///
/// </summary>
/// <param name="title"><inheritdoc cref="FileDialogSettings.Title" path="*"/></param>
/// <param name="filter"><inheritdoc cref="FileDialogSettings.Filter" path="*"/></param>
/// <param name="initialDirectory">The initial directory in the dialogue - if not supplied will get the directory path from the <paramref name="fileName"/></param>
/// <param name="fileName">The initial filename in the dialogue - if not supplied will use the filename from the <paramref name="settings"/> object </param>
public FileDialogSettings(string title, string filter = "All Files|*.*", string initialDirectory = null, string fileName = "")
{
Title = title ?? string.Empty;
Filter = filter ?? string.Empty;
InitializeFileNameAndDirectory(fileName, initialDirectory, null);
}
/// <summary>
/// Create a new Dialog using the settings of a previous dialogue
/// </summary>
/// <param name="settings">The previous dialogue whose settings will be adopted</param>
/// <param name="title">The title for the new dialogue - If not supplied will remain as string.empty</param>
/// <param name="fileName">The initial filename in the dialogue - if not supplied will use the filename from the <paramref name="settings"/> object </param>
/// <param name="initialDirectory">The initial directory in the dialogue - if not supplied will get the directory path from the <paramref name="fileName"/> </param>
public FileDialogSettings(FileDialogSettings settings, string title = "", string initialDirectory = null, string fileName = null)
{
//Use the settings from the provided settings object
AddExtension = settings?.AddExtension ?? true;
CheckPathExists = settings?.CheckPathExists ?? true;
if (!(settings?.CustomPlaces is null)) CustomPlaces = settings.CustomPlaces;
DefaultExt = settings?.DefaultExt ?? string.Empty;
DereferenceLinks = settings?.DereferenceLinks ?? true;
Filter = settings?.Filter ?? string.Empty;
FilterIndex = settings?.FilterIndex ?? 1;
ValidateNames = settings?.ValidateNames ?? true;
// Set the title for the new dialogue
Title = title ?? string.Empty;
InitializeFileNameAndDirectory(fileName, initialDirectory, settings.InitialDirectory);
}
/// <summary>
/// Initialize the FileName and the InitialDirectory based on the provided inputs <br/>
/// Used by the constructors that accept inputs for filename, initial directory.
/// </summary>
protected virtual void InitializeFileNameAndDirectory(string fileName, string initialDirectory, string settingsDirectory = null)
{
string fn = fileName.Trim() ?? string.Empty;
string dir = initialDirectory.Trim() ?? settingsDirectory ?? string.Empty;
bool HasFN = String.IsNullOrWhiteSpace(fn);
bool FNRooted = HasFN && Path.IsPathRooted(fn);
bool HasDir = String.IsNullOrWhiteSpace(dir);
//Decide on FileName first
if (!HasFN)
{
FileName = string.Empty;
InitialDirectory = dir;
}
else if (FNRooted)
{
// Get the Initial Directory from the rooted filename
FileName = fn;
InitialDirectory = Path.GetPathRoot(fileName);
}
else if (HasDir)
{
//FileName present, but not rooted & initial directory specified
InitialDirectory = dir;
FileName = Path.Combine(dir, fn);
}
else
{
//FileName present, not rooted, no initial directory
FileName = fn;
InitialDirectory = dir;
}
}
}
Using that setup, one could then do something like the following:
private FileDialogSettings defaultSettings;
private FileDialogSettings LastUsedDialogue;
public void OpenFile()
{
var Diag = new OpenFileDialogSettings2(LastUsedDialogue ?? defaultSettings, initialDirectory: "C:\MyDocuments");
// show the dialog, perform the actions
LastUsedDialogue = Diag;
}
public void SaveFile()
{
var Diag = new SaveFileDialogSettings(LastUsedDialogue ?? defaultSettings, fileName: LastUsedDialogue?.FileName ?? this.Name ); // Where this is the viewmodel or whatever maybe
// show the dialog, perform the actions
LastUsedDialogue = Diag;
}
Thanks for the examples, your intent is clearer now!
The reason for choosing an abstract base class is because that what they also do in the .NET Framework. Now, I cannot say that they haven't changed their mind about that and kept the inheritance structure only because of backward compatibility reasons, but what I can make a claim for is that if you've worked with open/save dialogs before, you would see the similarities in this codebase, and that's a good thing.
Regarding your suggestions, I'm quite sure that these changes would benefit you in your development, but I have a hard time assessing whether these changes would benefit the general audience of developers already using this codebase. I'd like to suggest that we leave this issue open and see whether any other developers chime in regarding its usefulness.
I thank you for your improvement suggestion, and hope that you have a splendid time with WPF development.
So after thinking about it some more, I do agree with your reply.
And In writing the code, I realized that base class instantiating isn't really required for my suggestion, only the introduction of the constructor that accepts the base class to inherit the parameters.
The only 'benefit' that instantiated base class would provide is to have a 'default' object to pass around for default settings, but a static factory method is better suited for that (which is actually exactly what I did in my code), since it doesn't keep an object hanging about in memory forever. So regarding that, I agree with keeping it abstract.
My theory in programming is about having that 'second breakfast'. Sure we can do it the standard way. But can we make it better? That's where this suggestion came from, even though it's exactly use case doesn't really apply to my current project.
If you choose to implement, I believe I've covered all the bases with the code. if not, no worries since my projects already hooked up using the static factory calls (the extensions I made )
(Also, it took a bit to get used to using the library, but once I did it's pretty great. Just needed to add a few extension method shortcuts for myself)
I love the second breakfast! 🥞
But just to give you my perspective as an open source maintainer. I'd love to add functionality, but then I've also had the experience of adding cool features that turned out to limit my choices. If I'd realized the limitations I'd never introduced the features, but then again, how could I have known? It's also the fact that a greater codebase increases the effort one has to put into future development, and new features often opens up for doing something more than one way, which again increases the test burden for every succeeding release. And all this while still having a healthy relationship with your daywork (which pays for the house) and your family.
This project is one of my oldest open source contributions, I've been developing and maintaining this codebase for more than 10 years, but am unfortunately gravitating towards moving this into a state where it's maintained but not developed. I hope you understand.
100% get it.
If anyone wants that functionality I proposed it's pretty easily added with the code I provided in this thread.
This repository has transitioned into a state where it's actively maintained but not actively developed. The README has been updated to reflect this. Thanks for the proposal but unfortunately this won't be implemented.