3
\$\begingroup\$

I have created a simple application that has grown a little bit. Though most likely it won't grow anymore, I have looked back and I realized that it has become a little monster in terms of how many almost identical properties and methods I have created. Please let me know how this could be made more manageable etc.

Purpose of the application**

Upload dll files to a remote computer. There are three types of files - workflow plugin, archive plugin, processing plugin. After some time, I have extended the app so that for each file type it can also have a config xml file.

Structure

User controls

To keep things simpler and more reusable, I have created a user control for uploader section, as this will repeat three times.

It has:

  • Two path boxes (for paths of .dll and XML files) - these are smaller usercontrols with browse/open selected file/display previous files' functionalities
  • Two upload buttons - one for .dll and one for XML
  • Two sets of two labels - to display modified date of local and server file for .dll and XML files - labels can change colors
  • One 'open folder' button - to open a server folder where a .dll file is
  • A fold/unfold button so that it could be collapsed
  • A checkbox for autoupload

enter image description here

As a result, this user control has got 17 dependency properties and 4 events:

  • Four dependency properties for paths (current path and list of previous paths for two types of files)
  • Four dependency properties for labels
  • Four dependency properties for label brushes
  • A couple of miscellaneous DPs for 'auto-uplad' checkbox, visibility, groupbox header etc
  • Four routed events - for all buttons clicks

Full XAML on dotnetfiddle, click on the View tab

MainWindow.xaml

Now, in my MainWindow.xaml file, apart from crap like status bar and menu bar, I need to define three UploaderControls. And this is where it starts being pretty monstrous (especially if I were to have e.g. 10 types of files to upload) For each of the controls I need a separate set of properties to be bound.

  • Four properties for paths, connected with application settings
  • Four properties for XML file labels and brushes
  • Four properties for .dll file labels and brushes

Apart from that there's a bunch of miscellaneous props for visbility, auto-uplad and event handlers. I am not really concerned about those.

     <controls:UploaderControl ControlHeader="Workflow plugin"
                              ContentVisibility="{Binding ElementName=ThisUc, Path=WorkflowUploaderVisibility}"
                              HideButtonClick="OnHideButtonClick"
                                       CurrentProvidedPath="{Binding ElementName=ThisUc, Path=CurrentWorkflowPluginPath, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}"
                                       PreviousPathsCollection="{Binding ElementName=ThisUc, Path=PreviousWorkflowPluginPathsCollection}"
                                       CurrentProvidedConfigPath="{Binding ElementName=ThisUc, Path=CurrentWorkflowPluginConfigPath, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}"
                                       PreviousConfigPathsCollection="{Binding ElementName=ThisUc, Path=PreviousWorkflowPluginConfigPathsCollection}"
                                       DockPanel.Dock="Top"
                                       OpenServerFolder="UploaderControl_OnOpenServerFolder"
                                       AutoUploadAfterBuild="{Binding ElementName=ThisUc,Path=WorkflowAutoUpload,Mode=TwoWay}"
                                       LocalLastModified="{Binding ElementName=ThisUc, Path=LocalWorkflowPluginLastModified}"
                                       LocalLastModifiedBrush="{Binding ElementName=ThisUc, Path=LocalWorkflowPluginLastModifiedBrush}"
                                       ServerLastModified="{Binding ElementName=ThisUc, Path=ServerWorkflowPluginLastModified}"
                                       ServerLastModifiedBrush="{Binding ElementName=ThisUc, Path=ServerWorkflowPluginLastModifiedBrush}"
                                 LocalConfigLastModified="{Binding ElementName=ThisUc, Path=LocalWorkflowPluginConfigLastModified}"
                                       LocalConfigLastModifiedBrush="{Binding ElementName=ThisUc, Path=LocalWorkflowConfigLastModifiedBrush}"
                                       ServerConfigLastModified="{Binding ElementName=ThisUc, Path=ServerWorkflowPluginConfigLastModified}"
                                       ServerConfigLastModifiedBrush="{Binding ElementName=ThisUc, Path=ServerWorkflowConfigLastModifiedBrush}"
                                       UploadPluginClick="UploadButton_Click"
                              UploadConfigClick="UploaderControl_OnUploadConfigClick"
                                       UploaderName="{Binding Source={x:Static tmsObjectsNames:TmsServiceNames.Workflow}}" />
    <controls:UploaderControl ControlHeader="Archiver plugin"
                              ContentVisibility="{Binding ElementName=ThisUc, Path=ArchiverUploaderVisibility}"
                              HideButtonClick="OnHideButtonClick"
                              CurrentProvidedConfigPath="{Binding ElementName=ThisUc, Path=CurrentArchiverPluginConfigPath, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}"
                                       PreviousConfigPathsCollection="{Binding ElementName=ThisUc, Path=PreviousArchiverPluginConfigPathsCollection}"
                                       AutoUploadAfterBuild="{Binding ElementName=ThisUc,Path=ArchiverAutoUpload, Mode=TwoWay}"
                                       CurrentProvidedPath="{Binding ElementName=ThisUc, Path=CurrentArchiverPluginPath, UpdateSourceTrigger=PropertyChanged, Mode=TwoWay}"
                                       OpenServerFolder="UploaderControl_OnOpenServerFolder"
                                       DockPanel.Dock="Top"
                                       PreviousPathsCollection="{Binding ElementName=ThisUc, Path=PreviousArchiverPluginPathsCollection}"
                                      
                                       LocalLastModified="{Binding ElementName=ThisUc, Path=LocalArchiverPluginLastModified}"
                                       LocalLastModifiedBrush="{Binding ElementName=ThisUc, Path=LocalArchiverPluginLastModifiedBrush}"
                                       ServerLastModified="{Binding ElementName=ThisUc, Path=ServerArchiverPluginLastModified}"
                                       ServerLastModifiedBrush="{Binding ElementName=ThisUc, Path=ServerArchiverPluginLastModifiedBrush}"
                                       
                                       LocalConfigLastModified="{Binding ElementName=ThisUc, Path=LocalArchiverPluginConfigLastModified}"
                                       LocalConfigLastModifiedBrush="{Binding ElementName=ThisUc, Path=LocalArchiverConfigLastModifiedBrush}"
                                       ServerConfigLastModified="{Binding ElementName=ThisUc, Path=ServerArchiverPluginConfigLastModified}"
                                       ServerConfigLastModifiedBrush="{Binding ElementName=ThisUc, Path=ServerArchiverConfigLastModifiedBrush}"
                                       
                                        UploadPluginClick="UploadButton_Click"
                                        UploadConfigClick="UploaderControl_OnUploadConfigClick"
                                       UploaderName="{Binding Source={x:Static tmsObjectsNames:TmsServiceNames.Archiver}}" />

C#

For all the dependency properties above I have separate properties that are almost identical, except they are for different uploader type (workflow, archive, processing). Do I really need it like that? I suppose not, but I don't know how to handle this better.

Sample properties

    #region Wokrlfow plugin ConfigPathBox //this one will be 'repeated' six times - 3x for different file types and 2x for dll and xml


private string _currentWorkflowPluginConfigPath = Settings.Default.CurrentWorkflowPluginConfigPath;
public string CurrentWorkflowPluginConfigPath
{
    get
    {
        return _currentWorkflowPluginConfigPath;
    }
    set
    {
        _currentWorkflowPluginConfigPath = value;
        Settings.Default.CurrentWorkflowPluginConfigPath = value;
        RaisePropertyChanged(nameof(CurrentWorkflowPluginConfigPath));
        InitializeWorkflowPluginUploader();
    }
}

private bool _prevWorkflowConfigPathsEventSubscribed;
private ObservableCollection<string> _previousWorkflowPluginConfigPathsCollection = Settings.Default.PreviousWorkflowPluginConfigPathsCollection ?? new ObservableCollection<string>();
public ObservableCollection<string> PreviousWorkflowPluginConfigPathsCollection
{
    get
    {
        if (!_prevWorkflowConfigPathsEventSubscribed)
        {
            _previousWorkflowPluginConfigPathsCollection.CollectionChanged += _previousWorkflowConfigPathsList_CollectionChanged;
            _prevWorkflowConfigPathsEventSubscribed = true;
        }
        return _previousWorkflowPluginConfigPathsCollection;
    }
    set { }
}

void _previousWorkflowConfigPathsList_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
    RaisePropertyChanged(nameof(PreviousWorkflowPluginConfigPathsCollection));
    Settings.Default.PreviousWorkflowPluginConfigPathsCollection = PreviousWorkflowPluginConfigPathsCollection;
}
#endregion



   #region Workflow // this is a section for last modified info along with brushes for coloring. Again, these will be 'repeated' six times.

private string _localWorkflowPluginLastModified;
public string LocalWorkflowPluginLastModified
{
    get
    {
        return _localWorkflowPluginLastModified;
    }
    set
    {
        _localWorkflowPluginLastModified = value;
        RaisePropertyChanged(nameof(LocalWorkflowPluginLastModified));
        Brush local;
        Brush server;
        ColorizeLabels(LocalWorkflowPluginLastModified, out local, ServerWorkflowPluginLastModified, out server);
        LocalWorkflowPluginLastModifiedBrush = local;
        ServerWorkflowPluginLastModifiedBrush = server;
    }
}


private string _serverWorkflowPluginLastModified;
public string ServerWorkflowPluginLastModified
{
    get
    {
        return _serverWorkflowPluginLastModified;
    }
    set
    {
        _serverWorkflowPluginLastModified = value;
        RaisePropertyChanged(nameof(ServerWorkflowPluginLastModified));
        Brush local;
        Brush server;
        ColorizeLabels(LocalWorkflowPluginLastModified, out local, ServerWorkflowPluginLastModified, out server);
        LocalWorkflowPluginLastModifiedBrush = local;
        ServerWorkflowPluginLastModifiedBrush = server;
    }
}

private Brush _localWorkflowPluginLastModifiedBrush;
public Brush LocalWorkflowPluginLastModifiedBrush
{
    get
    {
        return _localWorkflowPluginLastModifiedBrush;
    }
    set
    {
        _localWorkflowPluginLastModifiedBrush = value;
        RaisePropertyChanged(nameof(LocalWorkflowPluginLastModifiedBrush));
    }
}


private Brush _serverWorkflowPluginLastModifiedBrush;
public Brush ServerWorkflowPluginLastModifiedBrush
{
    get
    {
        return _serverWorkflowPluginLastModifiedBrush;
    }
    set
    {
        _serverWorkflowPluginLastModifiedBrush = value;
        RaisePropertyChanged(nameof(ServerWorkflowPluginLastModifiedBrush));
    }
}

#endregion

I think I am handling button click events pretty well with switches, so that all uploaders point to the same handler and then proper object is launched:

 private async void UploaderControl_OnUploadConfigClick(object sender, RoutedEventArgs e)
        {
            FileUploadInfo = "Preparing upload...";
            Application.Current.MainWindow.Cursor = Cursors.Wait;

            string pluginName = (sender as Controls.UploaderControl).UploaderName;
            switch (pluginName)
            {
                case TmsServiceNames.Workflow:
                    await WorkflowFileUploader?.UploadConfig(false);
                    break;
                case TmsServiceNames.Archiver:
                    await ArchiverFileUploader?.UploadConfig(false);
                    break;
                case TmsServiceNames.PptService:
                    await PptFileUploader?.UploadConfig(false);
                    break;
            }
            Application.Current.MainWindow.Cursor = Cursors.Arrow;
        }

I have a big problem with initialization of each of the FileUploader classes. For each of the three types I had to create a separate method:

 private void InitializeWorkflowPluginUploader()
        {
            if (CurrentWorkflowPluginPath.IsExistingFilePath() && ServicesManager.Services.ContainsKey(TmsServiceNames.Workflow))
            {
                IProgress<FileWatcherProgress> p = new Progress<FileWatcherProgress>(x =>
                {
                    LocalWorkflowPluginLastModified = x.LocalFileVersion ?? LocalWorkflowPluginLastModified;
                    ServerWorkflowPluginLastModified = x.ServerFileVersion ?? ServerWorkflowPluginLastModified;
                    LocalWorkflowPluginConfigLastModified = x.LocalConfigFileVersion ?? LocalWorkflowPluginConfigLastModified;
                    ServerWorkflowPluginConfigLastModified = x.ServerConfigFileVersion ?? ServerWorkflowPluginConfigLastModified;
                    FileUploadInfo = x.OverallInfo ?? FileUploadInfo;
                    if (x.PopupText != null)
                    {
                        LaunchUploaderPopup(x.PopupTitle, x.PopupText);
                    }
                });
                WorkflowFileUploader = new RemoteFileUploader(CurrentWorkflowPluginPath, CurrentWorkflowPluginConfigPath, new TmsFolderPaths().Workflow, WorkflowAutoUpload, ShowPopupAfterAutoUpload, ServicesManager.Services[TmsServiceNames.Workflow], p);
            }
        }

I would like to create all these in a single method, where I could pass a bunch of properties as parameters - in this case for example file paths and properties for status labels (LocalWorkflowPluginLastModified) etc, which are different for each file type.

So, summing up, while this is finished and will not grow any further, I would like to do a better structure next time, reduce the number of properties if possible. What would I do if I had to add 5 more file types, and then add 2 more labels to each of them?

\$\endgroup\$
2
  • 2
    \$\begingroup\$ Create a metadata class containing a superset of all the configuration properties. Pass a collection of that to the building method. Use constructors! Years later no one knows the correct setting combinations for ours. For complex properties - like a string CSV list to be parsed - write methods to encapsulate said concat & de-concatination. The future you will thank you. P.S. the "one method..." Keep the code modular, clear, etc. It's "OO all the way down". Ours set complex properties directly, in hundred plus instances, and it's a *&^$% to understand and maintain; \$\endgroup\$ – radarbob Mar 9 '16 at 2:38
  • \$\begingroup\$ @radarbob - thanks a lot - do you think you would find the time to post it as an answer with some code examples? Not sure how a metadata class would look etc etc.. \$\endgroup\$ – Bartosz Mar 9 '16 at 9:40
1
\$\begingroup\$

Response to a Comment

My comment:

Create a metadata class containing a superset of all the configuration properties. Pass a collection of that to the building method. Use constructors! Years later no one knows the correct setting combinations for ours. For complex properties - like a string CSV list to be parsed - write methods to encapsulate said concat & de-concatination. The future you will thank you. P.S. the "one method..." Keep the code modular, clear, etc. It's "OO all the way down". Ours set complex properties directly, in hundred plus instances, and it's a *&^$% to understand and maintain;

Response:

@radarbob - thanks a lot - do you think you would find the time to post it as an answer with some code examples? Not sure how a metadata class would look etc etc.. – Bartosz Mar 9 at 9:40


FilterRow class

These properties customize SQL that populates DataGrids and about a dozen other UI controls. Different controls use different property combinations while some properties are common to all. There are about 300 instances created throughout our code.

There is necessarily design infrastructure to make this work. For example note the enum use. There is a lot of base-class handling for sub-class instantiation overrides. Also subsequent sub-class' property value handling may be customized.

In our most awesome use of this, 3 simple overrides customizes a screen of complex Sort, Range, and Filtering options used for report generation.

dr.ColumnName         = "a DB table column name";
dr.ColumnTableName    = "a DB table name";
dr.ColumnDefaultValue = string.Empty;
dr.ColumnLabel        = "A Column Heading";
dr.ColumnDataType     = DataType.String.ToString();
dr.ColumnControlType  = ControlType.ComboBox.ToString();
dr.ColumnOperator     = SQLOperator.EQ.ToString();
dr.ColumnDefaultValue = "";
dr.ColumnChoices      = colA, colB, Colc;  // the SELECT columns
dr.ColumnHeader       = Aae, Bee, Cee; // corresponding datagrid column headings
dr.ColumnGrouping     = ReportGroupType.Filter.ToString();
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.