1
\$\begingroup\$

I am a self-taught programmer and would love any advice / suggestions. I wanted to implement some sort of n-tier Architecture, but not sure how to move all my code around.

My program will take movies from my hard drive and eventually allow me to view the results in a nice Netflix like UI.

The code below is just the beginning, you click Add Directory to add directories that contains movies and it will crawl the directories looking for types of video.

I am using Windows Form because a ton of my code is already in it and don't mind it. For my next project I will definitely try to use WPF.

To start, the Add Directory button.

    private void buttonAddDirectory_Click(object sender, EventArgs e)
    {
        FolderBrowserDialog folderBrowserDialog1 = new FolderBrowserDialog();
        if (folderBrowserDialog1.ShowDialog() == DialogResult.OK)
        {
            processSelectedPath(folderBrowserDialog1.SelectedPath);
        }
    }

Starts the processSelectedPath method.

    void processSelectedPath(string Path)
    {
        new Thread(() =>
        {
            Thread.CurrentThread.IsBackground = true;
            foreach(DataGridViewRow row in dgwVideoFolders.Rows)
            {
                if (row.Cells[0].Value.ToString() == Path)//check for duplicates
                    return;
            }       
            processFolders(Path, true);
        }).Start();
    }

Which starts a thread that does the crawling and does not block the main UI.

After it checks for duplicates, it will call processFolders method.

  public void processFolders(string Folder, bool UpdateSettings)
        {
            try 
            {
                //crawl the directory to find movies with bgw
                List<string> videosFound = FindMovies.CrawlDirectory(Folder, checkBoxIgnoreSamples.Checked, checkBoxReplacePeriods.Checked);

                if (videosFound == null)
                {
                    //MessageBox.Show("No videos found.");
                    return;
                }

            string[] rows = new string[] { Folder, videosFound.Count.ToString() };

            this.dgwVideoFolders.Invoke(new UIUpdaterDelegate(() =>
            {
                this.dgwVideoFolders.Rows.Add(rows);
            }));

            for (int i = 0; i < videosFound.Count; i++)
            {
                SettingsClass.SaveMovieToDisk(videosFound[i]);
                //   addToMoviesDatagrid(videosFound[i]);
            }

            if (UpdateSettings)
                SaveVideoDirectories();

            this.dgwMyVideos.Invoke(new UIUpdaterDelegate(() =>
            {
                this.dgwMyVideos.Refresh();
            }));
            this.dgwVideoFolders.Invoke(new UIUpdaterDelegate(() =>
            {
                this.dgwVideoFolders.Refresh();
            }));
        }
        catch(Exception e)
        {
            WriteToLogs(e.ToString());
        }
    }

The adding, refreshing to a datagrid and processing folder logic is all on the UI thread. Bad? Should the logic be moved to another class and just make a method on the UI for adding rows / refreshing the data grid?

Crawling Directory object.

   using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.IO;
using System.Text.RegularExpressions;

namespace Movie_Management
{
    class FindMovies
    {
        private static string[] AllowedVideoTypes = new string[] {"\\.avi", "\\.mp4", "\\.wmp", "\\.mkv", "\\.m4v" };
        private static long minVideoLength = 1000000;
        public static List<string> CrawlDirectory(string Path, bool ignoreSamples, bool replacePeriods)
        {

                List<string> videosFound = new List<string>();
                foreach (var file in Directory.GetFiles(Path))
                {
                    string replacement = file;

                    if (replacePeriods)
                    {
                        string endString = file.Substring(file.Length - 4);
                        replacement = file.Substring(0, file.Length - 4);
                        replacement = replacement.Replace(".", " ");
                        replacement = replacement + endString;

                        if (file != replacement)
                            File.Move(file, replacement);
                    }
                    bool bigEnough = false;
                    FileInfo videoInfo = new FileInfo(replacement);
                    if (videoInfo.Length > minVideoLength)
                        bigEnough = true;


                    if (IsVideo(replacement, ignoreSamples) && bigEnough)
                        videosFound.Add(replacement);

                }
                foreach (var dir in Directory.GetDirectories(Path))
                {

                    string cleanedDir = dir;
                    if (replacePeriods)
                    {
                        cleanedDir = dir.Replace(".", " ");

                        if (cleanedDir != dir)
                        {
                            try
                            {
                                Directory.Move(dir, cleanedDir);
                            }
                            catch (Exception)
                            {
                                break;
                            }

                        }

                    }

                    foreach (var file in Directory.GetFiles(cleanedDir))
                    {
                        string replacement = file;

                        if (replacePeriods)
                        {
                            string endString = file.Substring(file.Length - 4);
                            replacement = file.Substring(0, file.Length - 4);
                            replacement = replacement.Replace(".", " ");
                            replacement = replacement + endString;

                            try
                            {
                                if (file != replacement)
                                    File.Move(file, replacement);
                            }
                            catch (Exception)
                            {
                                break;
                            }
                        }
                        bool bigEnough = false;
                        FileInfo videoInfo = new FileInfo(replacement);
                        if (videoInfo.Length > minVideoLength)
                            bigEnough = true;


                        if (IsVideo(replacement) && bigEnough)
                            videosFound.Add(replacement);


                    }
                }
                return videosFound;

        }

        public static void SaveSettings(List<string> Folders, string Dir)
        {
            if (!Directory.Exists(Dir))
                Directory.CreateDirectory(Dir);

            string FilePath = Dir + "video_folders.dat";
            if (!File.Exists(FilePath))
                File.Create(FilePath).Dispose();

            StreamWriter file = new StreamWriter(FilePath);
            for (int i = 0; i < Folders.Count; i++)
            {
                file.WriteLine(Folders[i]);
            }
            file.Close();
        }


        public static string GetType(string input, bool ignoreSamples = false)
        {
            for (int i = 0; i < AllowedVideoTypes.Length; i++)
            {
                bool ignore = false;
                if (ignoreSamples)
                {
                    Match containsSamples = Regex.Match(input, "sample", RegexOptions.IgnoreCase);
                    if(containsSamples.Success)
                        ignore = true;

                }
               if(!ignore)
               {
                    Match rightType = Regex.Match(input, AllowedVideoTypes[i], RegexOptions.IgnoreCase);
                    if (rightType.Success)
                        return rightType.Value.ToString();
               }

            }
            return null;
        }

        private static bool IsVideo(string File, bool ignoreSamples = true)
        {
            for (int i = 0; i < AllowedVideoTypes.Length; i++)
            {
                bool ignore = false;
                if (ignoreSamples)
                {
                    Match containsSamples = Regex.Match(File, "sample", RegexOptions.IgnoreCase);
                    if (containsSamples.Success)
                    {
                        ignore = true;
                        return false;
                    }

                }
                if (!ignore)
                {
                    Match rightType = Regex.Match(File, AllowedVideoTypes[i], RegexOptions.IgnoreCase);
                    if (rightType.Success)
                        return true;
                }
            }
            return false;
        }

    }
}

The way that I store wether or not the check box is checked bugs me, I feel like there is a better way I don't know.

\$\endgroup\$
  • \$\begingroup\$ I need to fix the PascalCasing and camelCasing. \$\endgroup\$ – Illuminati Jan 29 '16 at 7:44
3
\$\begingroup\$

I wanted to implement some sort of n-tier Architecture

I doubt that you want to do that, since n-tier architectures focus on physical separation of concerns, which would for your scope not make much sense. In any case, it is always a good idea to logically separate your concerns.

Your question about how to structure the code depends on several things:

  1. Which concerns/responsibilities do you have?
  2. Which quality concerns do you have, e.g. maintainability, maximum reuse, etc.?
  3. What is likely to be changed or extended in the future?

Answering these conceptual kinds of questions is commonly known as design.

For producing clean code, readability and name expressiveness is important. Focusing on these properties is my personal opinion, what clean code is in general is an ongoing discussion in the field. However, for beginners, I'd suggest these to begin with.

Considering these more general comments should already change your thinking.

With respect to your code, here are my comments:

  1. The processSelectedPath method actually has two responsibilities: duplicate checking by using the DataGrid and actually processing the folders. Both responsbilities should be separated into two methods.
  2. Including the duplicate check that uses the datagrid view in a new thread is not a good idea, since the datagrid view is itself created in the UI thread.
  3. Since the UI should be only responsible for showing a view on your data, it should not act as the data representation itself, i.e. checking for duplicates should not be done by making use of the UI, but by using a data representation that is independent of the UI. This could be a class maintaining a list on which folders have been crawled already. The datagrid can be data bound to this list, since it is always up-to-date.
  4. An indication that a method has too many responsibilites is that you cannot find an expressive name for it as it's the case with "processFolders". "ProcessXXX" is in general a bad pattern for naming, since it can mean anything. In your case however, you a) look recursively for videos in a directory structure b) update the UI and c) persist the found videos. These should also go in their own methods.
  5. All behavior that is not related to UI updating should be put in separate classes and your UI Form should get a public method that accepts an object that is then displayed. This method is invoked by your business logic class and passed the information it needs. In this way the folder crawling does not need to know anything about UI and the interface between UI and business logic is clearly defined, i.e. it consists of a data object that has only the minimum data the UI needs to perform its job. For your interest, this is what is commonly known as Model View Presenter Pattern (MVP), if you want to further read on that.

All in all, your code structure should be driven by minimizing interfaces and modularizing responsibilities, since these are integral ingredients of good and maintainable software.

\$\endgroup\$
  • \$\begingroup\$ I doubt that you want to do that, since n-tier architectures focus on physical separation of concerns Since when does it refer to physical separation? I've often heard this term used to refer to the many different software layers that may exist, regardless of where that particular tier is physically. \$\endgroup\$ – RubberDuck Jan 30 '16 at 12:19
  • 1
    \$\begingroup\$ @RubberDuck Well, we're talking here about definitions that are not set in stone (yet). In my working environment, a tier is a physical separation, while layers are an architectural style that separate logically. In a sequential way this makes sense, because your defined layers can still be deployed to different physical devices (e.g. fat vs thin clients) and in this respect the term tier fills the gap in communication. \$\endgroup\$ – McMannus Jan 31 '16 at 19:37
  • \$\begingroup\$ I could see a tier vs. layer distinction @McMannus. \$\endgroup\$ – RubberDuck Jan 31 '16 at 19:38

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.