WPF wizard-like app

Question

I'm designing and developing wizard-like app for populating the database I created with data from different sources. It's my first attempt to design and develop well-structured object-oriented software from scratch (including database, web API + web site + client desktop app and this app for working with database). Since I don't have experience and before developed only small console school projects, I ran into a lot of problems and questions regarding design. I will describe my app in general to provide a better understanding of what it is and possible problems.

Scenario:

1. A user launches app and sees window where he can choose what operation he needs to perform on database.
2. Depending on which operation the user chose, he's presented with input prompt according to input data required for operation chosen. It may be prompted to choose file or type web address or both.
3. The user is asked to wait and he can see progress of chosen operation. Also he can cancel the operation.
4. The user can choose operation again.

Requirements:

For now each operation is insertion of a data set into the database from somewhere. Source may be a file of some format on local hard drive or web resource for example. Also multiple sources may be used for one operation(for cleaning, comparing or merging data as required to satisfy the schema of the database).

The main requirement and purpose of creating this app is the ability to rapidly and easily add new data-specific operation to develop the database, reuse infrastructure and already written code as much as possible. I have come to this conclusion after having written ~1000 lines of bad, duplicated procedural code and thinking a lot because it was easier and faster to write and run each operation as independent at first.

What I did:

Because each operation is different from another one in data source, data pre-processing and probably database connection provider or query method, I defined three main layers in my program:

Data reader class which encapsulates all methods required for reading data from different sources:

class Reader
{
    public string[] Read(string path)
    {
        return File.ReadAllLines(path);
    }

    public List<string[]> Read(string path1, string path2)
    {
        List<string[]> dataSet = new List<string[]>();

        var array1 = File.ReadAllLines(path1);
        var array2 = File.ReadAllLines(path2);
        dataSet.Add(array1);
        dataSet.Add(array2);

        return dataSet;
    }
}

Data parser classes which are operation-specific and take raw data, parse it, clean, format etc. and return formatted clean data set. One of them:

class WordDefinitionParser
{
    public DataTable Parse(List<string[]> data, IProgress<int> progress)
    {
        Dictionary<string, int> defIds = GetDefinitionIds(data[1]);

        DataTable dt = new DataTable();
        dt.TableName = "WordDefinition";
        dt.Columns.Add("Words_WordID");
        dt.Columns.Add("Definitions_DefinitionID");

        int onePercent = data[0].Length/100;
        int wordID = 0;
        int curPercent = 0;

        foreach (var item in data[0])
        {
            if (item == string.Empty) continue;
            wordID++;
            var splitLine = item.Split(':'); 
            var synsetsInfo = splitLine.Last().Split(';');
            foreach (var syn in synsetsInfo)
            {
                if (syn == string.Empty) continue;
                var synsetID = syn.Split(',').First();
                int definitionID = defIds[synsetID];
                DataRow row = dt.NewRow();
                row[0] = wordID;
                row[1] = definitionID;
                dt.Rows.Add(row);
            }

            if (wordID % onePercent == 0) { curPercent++; progress.Report(curPercent); }
        }

        progress.Report(100);
        return dt;
    }

    private Dictionary<string, int> GetDefinitionIds(string[] data)
    {
        var definitionIds = new Dictionary<string, int>();
        int id2 = 0;
        foreach (var item in data)
        {
            var id = item.TrimStart('g', '(').Substring(0, 9);
            id2++;
            definitionIds.Add(id, id2);
        }

        return definitionIds;
    }
}

And Repository class:

public class Repository
{
    public void Add<T>(IEnumerable<T> data, IProgress<int> progress) where T : class
    {
        var dbContext = new DictionaryContainer();
        dbContext.Configuration.ValidateOnSaveEnabled = false;
        dbContext.Configuration.AutoDetectChangesEnabled = false;
        int onePercent = data.Count()/100;
        int count = 0;
        int percent = 0;
        try
        {
            foreach (var item in data)
            {
                if (count % 100 == 0)
                {
                    dbContext.SaveChanges();
                    dbContext.Dispose();
                    dbContext = new DictionaryContainer();
                    dbContext.Configuration.AutoDetectChangesEnabled = false;
                    dbContext.Configuration.ValidateOnSaveEnabled = false;
                }
                dbContext.Set<T>().Add(item);
                count++;
                if(count % onePercent == 0) { percent++; progress.Report(percent); } 
            }

        }
        finally
        {
            progress.Report(100);
            if (dbContext != null)
            {
                dbContext.SaveChanges();
                dbContext.Dispose();
            }
        }
    }

    public void AddUsingSqlBulkCopy(DataTable data, IProgress<int> progress)
    {
        var connectionStr = ConfigurationManager.ConnectionStrings["Dictionary"].ConnectionString;
        using (SqlBulkCopy sbc = new SqlBulkCopy(connectionStr))
        {
            sbc.DestinationTableName = data.TableName;
            progress.Report(10);
            sbc.WriteToServer(data);
            progress.Report(100);
        }
    }

}

To implement the scenario I used content control and user controls.

MainWindow.xaml:

    <Window x:Class="DatabaseWriter.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:local="clr-namespace:DatabaseWriter"
        mc:Ignorable="d"
        Title="MainWindow" Height="350" Width="525">

        <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="*" />
            <RowDefinition Height="50" />
        </Grid.RowDefinitions>

        <ContentControl Name="View" Margin="20" />
        <StackPanel Grid.Row="1" Orientation="Horizontal" HorizontalAlignment="Right">
            <Button Name="Back" Content="Back" Width="80" Height="30" Margin="0,0,10,0" Click="Back_Click" />
            <Button Name="Next" Content="Next" Width="80" Height="30" Margin="0,0,10,0" Click="Next_Click" />
            <Button Name="Cancel" Content="Cancel" Width="80" Height="30" Margin="0,0,10,0" Click="Cancel_Click" />
        </StackPanel>
    </Grid>
</Window>

MainWindow code behind where is switching of views happening:

public partial class MainWindow : Window
{
    private int _currentPage;

    public MainWindow()
    {
        InitializeComponent();
        View.Content = new StartupView();
        _currentPage = 0;
    }

    private void Next_Click(object sender, RoutedEventArgs e)
    {
        View.Content = Router.CurrentRoute[++_currentPage];
    }

    private void Back_Click(object sender, RoutedEventArgs e)
    {
        View.Content = Router.CurrentRoute[--_currentPage];
    }

    private void Cancel_Click(object sender, RoutedEventArgs e)
    {
        throw new NotImplementedException();
    }
}

StartupView.xaml is the first page where user chooses operation:

    <UserControl x:Class="DatabaseWriter.Views.StartupView"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             xmlns:local="clr-namespace:DatabaseWriter.Views"
             mc:Ignorable="d" 
             d:DesignHeight="300" d:DesignWidth="333.835" Width="Auto" Height="Auto">

    <StackPanel>
        <TextBlock Height="50" Text="I want to insert into database(choose):" Padding="10" FontSize="18" />
        <ComboBox x:Name="comboBox" SelectedValuePath="Content" HorizontalAlignment="Left" Height="25" Margin="0,20,0,0" Width="300" SelectionChanged="comboBox_SelectionChanged">
            <ComboBoxItem>word</ComboBoxItem>
            <ComboBoxItem>definition</ComboBoxItem>
            <ComboBoxItem>word-definition relationship</ComboBoxItem>
        </ComboBox>
    </StackPanel>

</UserControl>

StartupView code behind:

public partial class StartupView : UserControl
{
    private string _operationType;

    public StartupView()
    {
        InitializeComponent();
        _operationType = string.Empty;
    }

    private void comboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
    {
        _operationType = comboBox.SelectedValue.ToString();
        Router.OperationType = _operationType;
    }
}

InputFilesView1.xaml is displayed if input for operation chosen by user is only one file:

    <UserControl x:Class="DatabaseWriter.Views.InputFilesView1"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             xmlns:local="clr-namespace:DatabaseWriter.Views"
             mc:Ignorable="d" 
             d:DesignHeight="300" d:DesignWidth="300" Width="Auto" Height="Auto">
    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="100" />
            <RowDefinition />
        </Grid.RowDefinitions>
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="110" />
            <ColumnDefinition />
        </Grid.ColumnDefinitions>

        <TextBlock Grid.ColumnSpan="2" />
        <Button Name="ChooseFile" Grid.Row="1" Content="Choose file" Height="30" Width="100" HorizontalAlignment="Left" Click="ChooseFile_Click" />
        <TextBox Grid.Row="1" Grid.Column="1" Height="30" />
    </Grid>
</UserControl>

Code behind:

public partial class InputFilesView1 : UserControl
{
    public string inputFilePath;

    public InputFilesView1()
    {
        InitializeComponent();
        inputFilePath = string.Empty;
    }

    private void ChooseFile_Click(object sender, RoutedEventArgs e)
    {
        OpenFileDialog openFileDialog = new OpenFileDialog();
        openFileDialog.Filter = "Text files(*.txt)|*.txt";

        if (openFileDialog.ShowDialog() == true)
        {
            inputFilePath = openFileDialog.FileName;
            Router.CurrentOperationModel.MapInputData(this);
        }
    }
}

InputFilesView2.xaml is for operations which require two input files:

    <UserControl x:Class="DatabaseWriter.Views.InputFilesView2"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             xmlns:local="clr-namespace:DatabaseWriter.Views"
             mc:Ignorable="d" 
             d:DesignHeight="300" d:DesignWidth="300" Width="Auto" Height="Auto">
    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition />
            <RowDefinition />
            <RowDefinition />
        </Grid.RowDefinitions>
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="110" />
            <ColumnDefinition />
        </Grid.ColumnDefinitions>
        <TextBlock Grid.ColumnSpan="2" />
        <Button Name="ChooseFile1" Grid.Row="1" Content="Choose file" Height="30" Width="100" HorizontalAlignment="Left" Click="ChooseFile1_Click" />
        <TextBox Grid.Row="1" Grid.Column="1" Height="30" />
        <Button Name="ChooseFile2" Grid.Row="2" Content="Choose file" Height="30" Width="100" HorizontalAlignment="Left" Click="ChooseFile2_Click" />
        <TextBox Grid.Row="2" Grid.Column="1" Height="30" />
    </Grid>
</UserControl>

Code behind:

public partial class InputFilesView2 : UserControl
{
    public string inputFilePath1;
    public string inputFilePath2;

    public InputFilesView2()
    {
        InitializeComponent();
        inputFilePath1 = string.Empty;
        inputFilePath2 = string.Empty;
    }

    private void ChooseFile1_Click(object sender, RoutedEventArgs e)
    {
        OpenFileDialog openFileDialog = new OpenFileDialog();
        openFileDialog.Filter = "Text files(*.txt)|*.txt";

        if (openFileDialog.ShowDialog() == true)
        {
            inputFilePath1 = openFileDialog.FileName;
            Router.CurrentOperationModel.MapInputData(this);
        }
    }

    private void ChooseFile2_Click(object sender, RoutedEventArgs e)
    {
        OpenFileDialog openFileDialog = new OpenFileDialog();
        openFileDialog.Filter = "WordNet files(*.pl)|*.pl";

        if (openFileDialog.ShowDialog() == true)
        {
            inputFilePath2 = openFileDialog.FileName;
            Router.CurrentOperationModel.MapInputData(this);
        }
    }
}

ProgressBarView.xaml is displayed for all operations and it's where operation's execution starts and progress displayed:

    <UserControl x:Class="DatabaseWriter.Views.ProgressBarView"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             xmlns:local="clr-namespace:DatabaseWriter.Views"
             mc:Ignorable="d" 
             d:DesignHeight="300" d:DesignWidth="300">

    <StackPanel VerticalAlignment="Center">
        <TextBlock Height="30" />
        <ProgressBar Name="ProgressBar" Minimum="0" Maximum="100" Height="30" Loaded="ProgressBar_Loaded" />
    </StackPanel>

</UserControl>

Code behind:

public partial class ProgressBarView : UserControl
{
    public ProgressBarView()
    {
        InitializeComponent();
    }

    private async void ProgressBar_Loaded(object sender, RoutedEventArgs e)
    {
        var progress = new Progress<int>(percent => ProgressBar.Value = percent);
        await Task.Run(() => Router.CurrentOperation.Execute(progress));
    }
}

Router class which is obviously responsible for mapping views with operation chosen and also used as single point of access to the operation:

static class Router
{
    private static Dictionary<string, List<Func<UserControl>>> _routes;

    private static Dictionary<string, Func<OperationModelBase>> _operationModels;

    private static string _operationType;

    public static string OperationType
    {
        get { return _operationType; }
        set {
            _operationType = value;
            ChangeRoute();
        }
    }

    public static List<UserControl> CurrentRoute { get; private set; }

    public static OperationModelBase CurrentOperation { get; private set; }

    static Router()
    {
        _routes = new Dictionary<string, List<Func<UserControl>>>();
        CurrentRoute = new List<UserControl>();
        _routes.Add("word", new List<Func<UserControl>>() {
            () => new StartupView(),
            () => new InputFilesView1(),
            () => new ProgressBarView() });
        _routes.Add("definition", new List<Func<UserControl>>() {
            () => new StartupView(),
            () => new InputFilesView1(),
            () => new ProgressBarView() });
        _routes.Add("word-definition relationship", new List<Func<UserControl>>() {
            () => new StartupView(),
            () => new InputFilesView2(),
            () => new ProgressBarView() });
        _operationModels = new Dictionary<string, Func<OperationModelBase>>();
        _operationModels.Add("word", () => new WordsOperationModel());
        _operationModels.Add("definition", () => new DefinitionsOperationModel());
        _operationModels.Add("word-definition relationship", () => new WordDefinitionOperationModel());
    }

    private static void ChangeRoute()
    {
        CurrentRoute.Clear();
        foreach (var item in _routes[OperationType])
        {
            var view = item();
            CurrentRoute.Add(view);
        }
        CurrentOperation = _operationModels[OperationType]();
    }
}

To provide flexibility in what operation can do and use as input data and avoid if/else, switch, duplicate code in the view I defined OperationModelBase class as interface:

public abstract class OperationModelBase
{
    public abstract void Execute(IProgress<int> p);

    public abstract void MapInputData(UserControl view);
}

And finally operation-specific OperationModel class. One of them:

class WordDefinitionOperationModel : OperationModelBase
{
    public string InputFilePath1 { get; set; }

    public string InputFilePath2 { get; set; }

    public override void Execute(IProgress<int> progress)
    {
        var reader = new Reader();
        var data = reader.Read(InputFilePath1, InputFilePath2);
        var parser = new WordDefinitionParser();
        var dataTable = parser.Parse(data, progress);
        var repo = new Repository();
        progress.Report(0);
        repo.AddUsingSqlBulkCopy(dataTable, progress);
    }

    public override void MapInputData(UserControl view)
    {
        if(view is InputFilesView2)
        {
            var v = (InputFilesView2)view;
            InputFilePath1 = v.inputFilePath1;
            InputFilePath2 = v.inputFilePath2;
        }
    }
}

My questions are:

1. How can I improve the design of my application?
2. What problems do you see and how can I solve them?
3. How can I bind user input from view to the class that needs that (Operation) another way than it is done now?

I would really appreciate any feedback.

Answer 1

I fail to see the point of your Reader class. And so do you, apparently, because your method Read(string path1, string path2) doesn't even use Read(string path). But then Read(string path) is merely doing return File.ReadAllLines(path);, so there's no point to it.

If you name something dataSet, I expect a DataSet, not a List<string[]>. I also fail to see the point of that "dataSet", since it relies on you remembering which string[] belongs to which path. Even if it would be a Dictionary<string, string> (with the path being the key), I still don't see the upside considering that reading the contents is as simple as File.ReadAllLines(path);.

---

WordDefinitionParser's Parse(List<string[]> data, IProgress<int> progress) is also an odd bird. data[1] and data[0] basically rely on magic numbers and conventions, a recipe for future failure. You continue this pattern when you do row[0] and row[1]: instead of meaningless numbers, use the column names.

Why not use string.IsNullOrEmpty() instead of if (item == string.Empty)?

Beware of variable names like defIds or curPercent, instead use proper nouns in full. synsetsInfo and syn are incomprehensible to me. You're also inconsistent: synsetID vs defId.

There's no need to define splitLine, just do it all in one go:

var synsetsInfo = item.Split(':').Last().Split(';');

---

InputFilesView1 and InputFilesView2 are bad names. I guess the "1" and "2" correspond to "if input for operation chosen by user is only one file" and "for operations which require two input files", but that doesn't convey this to me at all. Name the first one SingleInputFilePicker and the other MultipleInputFilePicker for instance.

Same for inputFilePath1 and inputFilePath2: I'd guess that each file is a specific one, e.g. one contains the denifitionIds mentioned elsewhere. So why not reflect that in their names?

Even your UI doesn't seem to provide even the slightest bit of information -- until you click the button to open the file dialog.

---

You say:

duplicate code in the view I defined OperationModelBase class as interface

Except I don't see an interface, I see an abstract class.

Answer 2

Scratching the surface...

---

The class Reader doesn't need two methods. It can do its job with only one:

class Reader
{
    public IList<string[]> Read(param string[] paths)
    {
        return paths.Select(File.ReadAllLines).ToList();
    }
}

I think in this case it's better to return an IList<T> rather then an IEnumerable<T> in order to prevent unintended multiple file reads.

---

public DataTable Parse(List<string[]> data, IProgress<int> progress)

Why does this method require a List<string[]> if internaly you only work with data[0]?

---

var dbContext = new DictionaryContainer();

This looks wrong. The variable name and the type don't match. If the DictionaryContainer really is a DbContext then it probably needs to be disposed... but wait! You do this inside the loop when you reach 100%. Why don't use the using(){} statement?

I also wonder why you are recreating the db-context after saving everything. You're going to dispose it in a second so what's the point?

If you need to report progress then make it simpler:

using(var context = new DictionaryContainer())
{
    // Setup the context first.
    context.Configuration.AutoDetectChangesEnabled = false;
    context.Configuration.ValidateOnSaveEnabled = false;

    // Add all items and report progress.
    foreach (var item in data)
    {            
        context.Set<T>().Add(item);
        count++;
        if(count % onePercent == 0) { percent++; progress.Report(percent); } 
    }

    // Save changes.
    context.SaveChanges();
    progress.Report(100);
}

---

Your usage of var and full type names is very inconsistant. Inside the same method you use both conventions.

---

private void comboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
{
    _operationType = comboBox.SelectedValue.ToString();
    Router.OperationType = _operationType;
}

You should use a property to get the selected operation-type. With C# 6 you could do:

public string SelectedOperationType => comboBox.SelectedValue?.ToString();

The other line doesn't look good either:

Router.OperationType = _operationType;

---

static classes like the Router are difficult to test. I suggest making it instantiabale and try to use Dependency Injection and an interface to pass its instance to controls that need to interact with it so that they are not dependant on it directly.

But I think you're reinventing the wheel with the Router. Check out the NavigationService Class - it might be exactly what you need.

public partial class StartupView : UserControl
{
    public StartupView()
    {
        InitializeComponent();
        _operationType = string.Empty;
    }

    public IRouter Router { get; set; }

    public string SelectedOperationType => comboBox.SelectedValue?.ToString();

    private void comboBox_SelectionChanged(object sender, SelectionChangedEventArgs e)
    {
        Router.OperationType = SelectedOperationType;
    }
}

or alternatively you can add a new SelectedOperationType event to the StartupView and handle it accordingly.

You may also try to create a command for it like SelectOperationTypeCommand.

---

You have two InputFilesViews and you use user-controls but why do we create user-control actually? So that we don't have to repeat ourselves and so that we can reuse them.

They are nearly identical. Keep only a single view but make its parts optional so that you can disable them. For example you could use the InputFilesView2 and disable the second file for the first instance.

---

Batch save (EDIT)

I oversaw the fact that it is a batch-save that saves each 100 items and not a percentage like I thought. In this case I suggest a more generic solution that allows you to reuse it.

I don't like the manual dispose and the new DbContext. It might go sideways if it crashes the context won't be disposed properly.

It is an extension that you can use on a colleciton of entities. It requires the size of the batch and a factory method for the context. The last parameter is for progress reporting.

Unlike the original solution this one works with double so it is able to report progress for collection that are smaller then 100 items.

Internally it uses two lambdas: one for checking if the batch should be saved and the other one for progress reporting. The context is wrapped with a using statement.

static class EnumerableExtensions
{
    public static void BatchSave<T, TContext>(
        this IList<T> items,
        int batchSize, Func<TContext> createContext,
        IProgress<double> progress)
        where T : class
        where TContext : DbContext
    {
        var count = 0;
        var lastPercentage = 0d;

        var saveBatch = new Func<bool>(() => ++count % batchSize == 0);

        var reportProgress = new Action(() =>
        {
            var percentage = Math.Round((double)count / (double)items.Count * 100d, 1);
            if (percentage > lastPercentage)
            {
                progress.Report(percentage);
                lastPercentage = percentage;
            }
        });

        var enumerator = items.GetEnumerator();
        while (enumerator.MoveNext())
        {
            using (var context = createContext())
            {
                context.Configuration.AutoDetectChangesEnabled = false;
                context.Configuration.ValidateOnSaveEnabled = false;

                do
                {
                    context.Set<T>().Add(enumerator.Current);                       
                    reportProgress();
                } while (!saveBatch() && enumerator.MoveNext());
                context.SaveChanges();                  
            }
        }
        reportProgress();
    }
}

Example:

var items = Enumerable.Range(0, 23).Select(x => x.ToString()).ToArray();

var batchSize = 10;

var progress = new Progress<double>(percent => Console.WriteLine($"Progress {percent}"));
items.BatchSave(batchSize, () => new YourContext(), progress);