HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

File creation program

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fileprogramcreation

Problem

I have written a program that is very simple, with a very specific purpose. However, having written it in approximately 2 hours, (with a bit of cut/paste from some of my other programs and MSDN) it's very ugly.

For example, the main part of the program is held entirely within one big using block, when it really doesn't need to be.

Also, while for the initial purpose of this program is to create placeholder files, the current .pdf files are not in the correct format, which is fine. However, is it possible to create pdf's in the correct format?

Can anyone give me any suggestions on how to neaten this up?

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.IO;
using Microsoft.VisualBasic.FileIO;

namespace File_Creation_Program
{
class Program
{
[STAThread]
static void Main(string[] args)
{
// Clears console in case program has been re-run
Console.Clear();

List fileNames = new List();

Console.WriteLine("Please select 1 or more CSV files to be converted for fle creation.");
Console.WriteLine();

// "Multiselect" prperty allows more than 1 file to be selected at a time.
using (OpenFileDialog openFileDialog1 = new OpenFileDialog())
{
openFileDialog1.Title = "Open file";
openFileDialog1.CheckFileExists = true;
openFileDialog1.CheckPathExists = true;
openFileDialog1.DefaultExt = "csv";
openFileDialog1.Filter = "CSV Files (.csv)|.csv|All files (.)|.";
openFileDialog1.Multiselect = true;
openFileDialog1.FilterIndex = 1;
openFileDialog1.RestoreDirectory = true;
openFileDialog1.ReadOnlyChecked = true;
openFileDialog1.ShowReadOnly = true;

// If the search for the file is OK (i.e. the file

Solution

The first thing that I have noticed is the recursion in your Return method.

It is simpler just have a do while loop inside the Main method. Extract the details into separate methods and it should look like this:

private static void Main(string[] args)
    {
        bool continueProcessing;
        do
        {
            var fileNames = GetSourceFileNames();
            foreach (var f in fileNames)
            {
                var folderName = GetDestinationFolder(f);
                foreach (var line in File.ReadAllLines(f))
                {
                    foreach (var field in line.Split(','))
                    {
                        CreatePlaceholderPDFs(folderName, field);
                    }
                }
            }
            continueProcessing = AskToContinue();

        } while (continueProcessing);
    }


Your Return method is now named AskToContinue. It returns true if the user wants to continue.

private static bool AskToContinue()
    {
        while (true)
        {
            Console.WriteLine("Would you like to create more files? Y/N");
            var option = Console.ReadLine() ?? "";
            switch (option.ToUpper())
            {
                case "Y":
                case "YES":
                    return true;
                case "N":
                case "NO":
                    return false;
                default:
                    Console.WriteLine("Not a valid input... Please enter 'Yes' or 'No'");
                    break;
            }    
        }
    }


When handling external resources (files, windows resources, connections, etc) it's a good idea to have them opened and disposed on the same method. That's what the GetSourceFileNames and GetDestinationFolders method do now. (deleted a few lines - just left the important bits)

private static IEnumerable GetSourceFileNames()
    {
        using (var openFileDialog1 = new OpenFileDialog())
        {
            // set up openFileDialog1.Title = "Open file";
            if (openFileDialog1.ShowDialog() == DialogResult.OK)
            {
                return openFileDialog1.FileNames;
            }
        }
        return new string[0];
    }

    private static string GetDestinationFolder(string filename)
    {
        using (var saveFile = new FolderBrowserDialog())
        {
            if (saveFile.ShowDialog() == DialogResult.OK)
            {
                return saveFile.SelectedPath;
            }
        }
        return string.Empty;
    }


These methods open the resource, gets the data, closes the resource and then returns the data to the caller.

The final part is the method for creating the placeholder PDFs

private static void CreatePlaceholderPDFs(string folderName, string field)
    {
        PrintMessage("Working...");

        try
        {
            var fileName = Path.Combine(folderName, field);
            using (var writer = File.CreateText(fileName))
            {
                writer.WriteLine("Place holder file for Hard Copy entry");
                writer.Close();
            }
        }
        catch (Exception e)
        {
            Console.WriteLine("File not created - " + e.Message);
        }
    }

Code Snippets

private static void Main(string[] args)
    {
        bool continueProcessing;
        do
        {
            var fileNames = GetSourceFileNames();
            foreach (var f in fileNames)
            {
                var folderName = GetDestinationFolder(f);
                foreach (var line in File.ReadAllLines(f))
                {
                    foreach (var field in line.Split(','))
                    {
                        CreatePlaceholderPDFs(folderName, field);
                    }
                }
            }
            continueProcessing = AskToContinue();

        } while (continueProcessing);
    }
private static bool AskToContinue()
    {
        while (true)
        {
            Console.WriteLine("Would you like to create more files? Y/N");
            var option = Console.ReadLine() ?? "";
            switch (option.ToUpper())
            {
                case "Y":
                case "YES":
                    return true;
                case "N":
                case "NO":
                    return false;
                default:
                    Console.WriteLine("Not a valid input... Please enter 'Yes' or 'No'");
                    break;
            }    
        }
    }
private static IEnumerable<string> GetSourceFileNames()
    {
        using (var openFileDialog1 = new OpenFileDialog())
        {
            // set up openFileDialog1.Title = "Open file";
            if (openFileDialog1.ShowDialog() == DialogResult.OK)
            {
                return openFileDialog1.FileNames;
            }
        }
        return new string[0];
    }

    private static string GetDestinationFolder(string filename)
    {
        using (var saveFile = new FolderBrowserDialog())
        {
            if (saveFile.ShowDialog() == DialogResult.OK)
            {
                return saveFile.SelectedPath;
            }
        }
        return string.Empty;
    }
private static void CreatePlaceholderPDFs(string folderName, string field)
    {
        PrintMessage("Working...");

        try
        {
            var fileName = Path.Combine(folderName, field);
            using (var writer = File.CreateText(fileName))
            {
                writer.WriteLine("Place holder file for Hard Copy entry");
                writer.Close();
            }
        }
        catch (Exception e)
        {
            Console.WriteLine("File not created - " + e.Message);
        }
    }

Context

StackExchange Code Review Q#121498, answer score: 4

Revisions (0)

No revisions yet.