Bridge or Factory and How

Question

I'm trying to learn patterns and I've got a job that is screaming for a pattern, I just know it but I can't figure it out. I know the filter type is something that can be abstracted and possibly bridged. I'M NOT LOOKING FOR A CODE REWRITE JUST SUGGESTIONS. I'm not looking for someone to do my job. I would like to know how patterns could be applied to this example.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Data;
using System.IO;
using System.Xml;
using System.Text.RegularExpressions;

namespace CopyTool
{
    class CopyJob
    {
        public enum FilterType
        { TextFilter, RegExFilter, NoFilter }
        public FilterType JobFilterType { get; set; }

        private string _jobName;
        public string JobName { get { return _jobName; } set { _jobName = value; } }

        private int currentIndex;
        public int CurrentIndex { get { return currentIndex; } }

        private DataSet ds;

        public int MaxJobs { get { return ds.Tables["Job"].Rows.Count; } }

        private string _filter;
        public string Filter { get { return _filter; } set { _filter = value; } }

        private string _fromFolder;
        public string FromFolder
        {
            get { return _fromFolder; }
            set
            {
                if (Directory.Exists(value))
                { _fromFolder = value; }
                else
                { throw new DirectoryNotFoundException(String.Format("Folder not found: {0}", value)); }
            }
        }

        private List<string> _toFolders;
        public List<string> ToFolders { get { return _toFolders; } }

        public CopyJob()
        {
            Initialize();
        }

        private void Initialize()
        {
            if (ds == null)
            { ds = new DataSet(); }
            ds.ReadXml(Properties.Settings.Default.ConfigLocation);
            LoadValues(0);
        }

        public void Execute()
        {
            ExecuteJob(FromFolder, _toFolders, Filter, JobFilterType);
        }

        public void ExecuteAll()
        {
            string OrigPath;
            List<string> DestPaths;
            string FilterText;
            FilterType FilterWay;
            foreach (DataRow rw in ds.Tables["Job"].Rows)
            {
                OrigPath = rw["FromFolder"].ToString();
                FilterText = rw["FilterText"].ToString();
                switch (rw["FilterType"].ToString())
                {
                    case "TextFilter":
                        FilterWay = FilterType.TextFilter;
                        break;
                    case "RegExFilter":
                        FilterWay = FilterType.RegExFilter;
                        break;
                    default:
                        FilterWay = FilterType.NoFilter;
                        break;
                }
                DestPaths = new List<string>();
                foreach (DataRow crw in rw.GetChildRows("Job_ToFolder"))
                {
                    DestPaths.Add(crw["FolderPath"].ToString());
                }
                ExecuteJob(OrigPath, DestPaths, FilterText, FilterWay);
            }
        }

        private void ExecuteJob(string OrigPath, List<string> DestPaths, string FilterText, FilterType FilterWay)
        {
            FileInfo[] files;
            switch (FilterWay)
            {
                case FilterType.RegExFilter:
                    files = GetFilesByRegEx(new Regex(FilterText), OrigPath);
                    break;
                case FilterType.TextFilter:
                    files = GetFilesByFilter(FilterText, OrigPath);
                    break;
                default:
                    files = new DirectoryInfo(OrigPath).GetFiles();
                    break;
            }
            foreach (string fld in DestPaths)
            {
                CopyFiles(files, fld);
            }
        }

        public void MoveToJob(int RecordNumber)
        {
            Save();
            LoadValues(RecordNumber - 1);
        }

        public void AddToFolder(string folderPath)
        {
            if (Directory.Exists(folderPath))
            { _toFolders.Add(folderPath); }
            else
            { throw new DirectoryNotFoundException(String.Format("Folder not found: {0}", folderPath)); }
        }

        public void DeleteToFolder(int index)
        {
            _toFolders.RemoveAt(index);
        }

        public void Save()
        {
            DataRow rw = ds.Tables["Job"].Rows[currentIndex];
            rw["JobName"] = _jobName;
            rw["FromFolder"] = _fromFolder;
            rw["FilterText"] = _filter;
            switch (JobFilterType)
            {
                case FilterType.RegExFilter:
                    rw["FilterType"] = "RegExFilter";
                    break;
                case FilterType.TextFilter:
                    rw["FilterType"] = "TextFilter";
                    break;
                default:
                    rw["FilterType"] = "NoFilter";
                    break;
            }
            DataRow[] ToFolderRows = ds.Tables["Job"].Rows[currentIndex].GetChildRows("Job_ToFolder");
            for (int i = 0; i <= ToFolderRows.GetUpperBound(0); i++)
            {
                ToFolderRows[i].Delete();
            }
            foreach (string fld in _toFolders)
            {
                DataRow ToFolderRow = ds.Tables["ToFolder"].NewRow();
                ToFolderRow["JobId"] = ds.Tables["Job"].Rows[currentIndex]["JobId"];
                ToFolderRow["Job_Id"] = ds.Tables["Job"].Rows[currentIndex]["Job_Id"];
                ToFolderRow["FolderPath"] = fld;
                ds.Tables["ToFolder"].Rows.Add(ToFolderRow);
            }
        }

        public void Delete()
        {
            ds.Tables["Job"].Rows.RemoveAt(currentIndex);
            LoadValues(currentIndex++);
        }

        public void MoveNext()
        {
            Save();
            currentIndex++;
            LoadValues(currentIndex);
        }

        public void MovePrevious()
        {
            Save();
            currentIndex--;
            LoadValues(currentIndex);
        }

        public void MoveFirst()
        {
            Save();
            LoadValues(0);
        }

        public void MoveLast()
        {
            Save();
            LoadValues(ds.Tables["Job"].Rows.Count - 1);
        }

        public void CreateNew()
        {
            Save();
            int MaxJobId = 0;
            Int32.TryParse(ds.Tables["Job"].Compute("Max(JobId)", "").ToString(), out MaxJobId);
            DataRow rw = ds.Tables["Job"].NewRow();
            rw["JobId"] = MaxJobId + 1;
            ds.Tables["Job"].Rows.Add(rw);
            LoadValues(ds.Tables["Job"].Rows.IndexOf(rw));
        }

        public void Commit()
        {
            Save();
            ds.WriteXml(Properties.Settings.Default.ConfigLocation);
        }

        private void LoadValues(int index)
        {
            if (index > ds.Tables["Job"].Rows.Count - 1)
            { currentIndex = ds.Tables["Job"].Rows.Count - 1; }
            else if (index < 0)
            { currentIndex = 0; }
            else
            { currentIndex = index; }
            DataRow rw = ds.Tables["Job"].Rows[currentIndex];
            _jobName = rw["JobName"].ToString();
            _fromFolder = rw["FromFolder"].ToString();
            _filter = rw["FilterText"].ToString();
            switch (rw["FilterType"].ToString())
            {
                case "TextFilter":
                    JobFilterType = FilterType.TextFilter;
                    break;
                case "RegExFilter":
                    JobFilterType = FilterType.RegExFilter;
                    break;
                default:
                    JobFilterType = FilterType.NoFilter;
                    break;
            }
            if (_toFolders == null)
                _toFolders = new List<string>();
            _toFolders.Clear();
            foreach (DataRow crw in rw.GetChildRows("Job_ToFolder"))
            {
                AddToFolder(crw["FolderPath"].ToString());
            }
        }

        private static FileInfo[] GetFilesByRegEx(Regex rgx, string locPath)
        {
            DirectoryInfo d = new DirectoryInfo(locPath);
            FileInfo[] fullFileList = d.GetFiles();
            List<FileInfo> filteredList = new List<FileInfo>();
            foreach (FileInfo fi in fullFileList)
            {
                if (rgx.IsMatch(fi.Name))
                {
                    filteredList.Add(fi);
                }
            }
            return filteredList.ToArray();
        }

        private static FileInfo[] GetFilesByFilter(string filter, string locPath)
        {
            DirectoryInfo d = new DirectoryInfo(locPath);
            FileInfo[] fi = d.GetFiles(filter);
            return fi;
        }

        private void CopyFiles(FileInfo[] files, string destPath)
        {
            foreach (FileInfo fi in files)
            {
                bool success = false;
                int i = 0;
                string copyToName = fi.Name;
                string copyToExt = fi.Extension;
                string copyToNameWithoutExt = Path.GetFileNameWithoutExtension(fi.FullName);
                while (!success && i < 100)
                {
                    i++;
                    try
                    {
                        if (File.Exists(Path.Combine(destPath, copyToName)))
                            throw new CopyFileExistsException();
                        File.Copy(fi.FullName, Path.Combine(destPath, copyToName));
                        success = true;
                    }
                    catch (CopyFileExistsException ex)
                    {
                        copyToName = String.Format("{0} ({1}){2}", copyToNameWithoutExt, i, copyToExt);
                    }
                }
            }
        }
    }
    public class CopyFileExistsException : Exception
    {
        public string Message;
    }

}

Answer 1

This code is also "screaming" to be broken down into smaller more specialized objects.

Your CopyJob object seems to be more of a manager of a list of jobs. I would maybe change the name of this to CopyJobManager or something. You could then have CopyJob be the base class for the different filter types. The common code, Execute() for example, would be defined in the base class, and the custom behavior, Filtering for example, would be handled in the derived classes. You would have a TextFilterCopyJob, a RegExFilterCopyJob, and a NoFilterCopyJob.

Where the Factory pattern could come into play is when you're building a list of CopyJobs. You could have a CopyJobFactory object that takes in a row from your dataset and returns the proper child version of CopyJob. CopyJobManager would then do its operations on a list of CopyJobs instead of a list of dataset rows.

Answer 2

There's nothing wrong with using a switch statement like you have. It's not screaming for any design pattern other than that you can put it in a function so that you don't have the same switch twice.

The switch will be faster than using reflection, and the problem you're trying to solve doesn't really require the Factory pattern.

Answer 3

Here's some of what I did to implement a Factory pattern

First, I created an interface for the filter:

interface IFileFilter
{
    string GetFilterName();
    string GetFilterReadableName();
    FileInfo[] GetFilteredFiles(string path, string filter);
}

then I created sub-filter classes for this interface:

class RegExFileFilter : IFileFilter

{
    #region IFileFilter Members

    public string GetFilterName()
    {
        return "RegExFilter";
    }

    public string GetFilterReadableName()
    {
        return "RegEx Filter";
    }

    public FileInfo[] GetFilteredFiles(string path, string filter)
    {
        DirectoryInfo d = new DirectoryInfo(path);
        FileInfo[] fullFileList = d.GetFiles();
        List<FileInfo> filteredList = new List<FileInfo>();
        Regex rgx = new Regex(filter);
        foreach (FileInfo fi in fullFileList)
        {
            if (rgx.IsMatch(fi.Name))
            {
                filteredList.Add(fi);
            }
        }
        return filteredList.ToArray();
    }

    #endregion
}
class TextFileFilter : IFileFilter
{
    #region IFileFilter Members

    public string GetFilterName()
    {
        return "TextFilter";
    }

    public string GetFilterReadableName()
    {
        return "Text Filter";
    }

    public FileInfo[] GetFilteredFiles(string path, string filter)
    {
        DirectoryInfo d = new DirectoryInfo(path);
        FileInfo[] fi = d.GetFiles(filter);
        return fi;
    }

    #endregion
}
class NoFileFilter : IFileFilter
{
    #region IFileFilter Members

    public string GetFilterName()
    {
        return "TextFilter";
    }

    public string GetFilterReadableName()
    {
        return "Text Filter";
    }

    public FileInfo[] GetFilteredFiles(string path, string filter)
    {
        DirectoryInfo d = new DirectoryInfo(path);
        FileInfo[] fi = d.GetFiles(filter);
        return fi;
    }

    #endregion
}

Then I created a Factory:

public static IFileFilter FileFilter(string filterName)
{
    switch (filterName)
    {
        case "Text Filter":
            return new TextFileFilter();
        case "RegEx Filter":
            return new RegExFileFilter();
        default:
            return new NoFileFilter();
    }
}

Answer 4

I would suggest the following:

Refactor the switch statements (as @Jordan mentioned)

Add an extension method to convert the FilterType enum into an int and save that to the database rather than a string. E.g.

public static class FilterTypeExtensions
{
    public static int AsNumeric(this FilterType filterType)
    {
        return (int)filterType;
    }
}

As a minor point, the single line braces are horrible, either drop the braces or use proper spacing/indentation. :)

Answer 5

Whenever I see Swithcs or bricks of Ifs, I jump to the conclusion that atleast a strategy pattern could be created.

a clean and easy way to set one up, is use a dictionary<>

in your case your going to want a key value based on the filterName your cases relate to, and the value will be a new object of the filters.

now you can merely give the string to the dictionarys TryGetValue method and have it retrieve the correct filter object for you, boom!

Now you can encapsulate the mapping of the filters <--> Strings, and keep the logic and use of the filters from having to see the logic of retrieving the correct object!