patterncsharpMinor
File creation program
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
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
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:
Your Return method is now named AskToContinue. It returns true if the user wants to continue.
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)
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
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.