I was looking for ideas on improving the following code
static void Main(string[] args)
{
bool validInput1 = false;
string input1 = string.Empty;
bool validInput2 = false;
string input2 = string.Empty;
bool validFilePath = false;
string filePath = string.Empty;
try
{
Console.WriteLine("Import process started.");
while (!validFilePath)
{
Console.Write("Please enter the full path to the file you would like to import: ");
filePath = Console.ReadLine().Replace("\"","");
if (File.Exists(filePath))
validFilePath = true;
}
while (!validInput1)
{
Console.Write("Enter a valid eventID the import file: ");
input1 = Console.ReadLine();
if (ValidEventID(input1.Trim().Length))
validInput1 = true;
}
while (!validInput2)
{
Console.Write("Enter a valid import type code: ");
input2 = Console.ReadLine();
if (input2.Trim().ToUpper() == "EX" || input2.Trim().ToUpper() == "EL")
validInput2 = true;
}
var records = Utilities.ParseCSV(filePath);
var import = new Import
{
EventId = input1,
ImportType = input2
};
i开发者_C百科mport.ImportEventDates(records);
Console.WriteLine("Import process completed.");
Console.ReadLine();
}
catch (Exception ex)
{
Console.WriteLine("Error encountered");
Console.WriteLine(ex.ToString());
Console.ReadLine();
}
}
Thanks in advance for any help
I'd write a simple method to retrieve and validate user input :
public static string PromptUntilValid(string promptText, Func<string, bool> validationPredicate)
{
string input;
do
{
Console.Write(promptText);
input = Console.ReadLine();
} while (!validationPredicate(input))
return input;
}
This would allow your code to be refactored as follows :
...
filePath = PromptUntilValid(
"Please enter the full path to the file you would like to import: ",
s => File.Exists(s));
input1 = PromptUntilValid(
"Enter a valid eventID the import file: ",
s => ValidEventID(s.Trim().Length));
input2 = PromptUntilValid(
"Enter a valid import type code: ",
s => s.Trim().ToUpper() == "EX" || s.Trim().ToUpper() == "EL");
...
You could try taking the while loops out and putting them each in their own functions which return the valid filepath and inputs and throw exceptions if they're caught. For example:
public string FindValidFilepath()
{
string filePath = string.Empty
try{
while (!validFilePath)
{
Console.Write("Please enter the full path to the file you would like to import: ");
filePath = Console.ReadLine().Replace("\"","");
if (File.Exists(filePath))
validFilePath = true;
}
} catch (Exception ex) {
throw;
}
return filePath
}
and call it from the Main function. Other things you can do which will reduce errors if you have to add code is to put curly braces, {, around the code within your If statements. While not having them is syntactically legal, it will keep an error from popping up later. I have been bitten by a mistake like that in the past.
EDIT: The rethrowing of this exception is so that the original Try-Catch block can stay in place. Also, I've learned that "throw ex" loses the stack trace but simply "throw" keeps it. I've corrected this issue.
EDIT 2: One more thing I thought of, try catching specific exceptions and outputting a specific error explaining what went wrong to the user so they can better understand the problem. Most users don't understand a stack trace unless outputting the stack trace is a requirement.
Also, please excuse any small syntax errors, I'm more familiar with Java than C#.
精彩评论