debugcsharpModerate
Replace strings in a file
Viewed 0 times
filestringsreplace
Problem
I had to create an executable to search and replace strings in a file. This is to be used in my installer for text file manipulation. I have various placeholders in configuration files that I need to replace with proper values after querying the end user system.
I have written the following code. This being my first real experience with C#, I do not expect the code to be any good. I have come up with this code based upon a couple of hours of reading MSDN and stackoverflow. I needed someone to review this. There is no one greater than the community to do this.
Please review this and let me know what modification are necessary.
I am specifically concerned about the exception handling.
I have included two try-catch blocks.
My logic:
If I have all operations under one try-catch block, the failure of one process will block the execution of all others. That is, if my
has an exception and if I have
also in the same try block, it will not execute and I will not have any logs.
Also, if I have the
in the catch block, any exception in that method cannot be caught. Yes I can have try-catch in the WriteLog method but I see no problems in my approach too.
So first, I have a try-catch for file manipulation and then another for log write.
Please ignore the log path(hardcoded as D:).
```
class Program
{
static int Main(string[] args)
{
//args[0] = The file that needs modification
//args[1] = The string to replace
//args[2] = The string with which to replace args[1]
int intStatus = 0;
string strException = "";
//Get out immediately if no/improper arguments are found
if (args.Length < 3)
{
intStatus = 1;
return intStatus;
}
try
{
StreamReader str
I have written the following code. This being my first real experience with C#, I do not expect the code to be any good. I have come up with this code based upon a couple of hours of reading MSDN and stackoverflow. I needed someone to review this. There is no one greater than the community to do this.
Please review this and let me know what modification are necessary.
I am specifically concerned about the exception handling.
I have included two try-catch blocks.
My logic:
If I have all operations under one try-catch block, the failure of one process will block the execution of all others. That is, if my
text = Regex.Replace(text, args[1], args[2]);has an exception and if I have
WriteLog(args[0], args[1], args[2], strException, intStatus);also in the same try block, it will not execute and I will not have any logs.
Also, if I have the
WriteLog(args[0], args[1], args[2], strException, intStatus);in the catch block, any exception in that method cannot be caught. Yes I can have try-catch in the WriteLog method but I see no problems in my approach too.
So first, I have a try-catch for file manipulation and then another for log write.
Please ignore the log path(hardcoded as D:).
```
class Program
{
static int Main(string[] args)
{
//args[0] = The file that needs modification
//args[1] = The string to replace
//args[2] = The string with which to replace args[1]
int intStatus = 0;
string strException = "";
//Get out immediately if no/improper arguments are found
if (args.Length < 3)
{
intStatus = 1;
return intStatus;
}
try
{
StreamReader str
Solution
Use the
Use an
Since the status has 3 states, use an
Why do you try-catch while logging?
If you don't see any particular possibility for the log to fail, then simply assume it and don't try-catch the logging.
If you want the logging to no matter what not break the application with exceptions, and you cannot fix those exceptions (why?), then I would suggest putting the try-catch inside the logging method.
Main - exception should catch, log, and return
I would have the exception log the error and return, instead of continuing normally.
Alternatively, I would catch, log, and re-throw:
Use camelCase for local variables
Using a starting underscore (
Follow the de-facto conventions. (See e.g. https://stackoverflow.com/questions/2976627/on-c-sharp-naming-conventions-for-member-variables )
Avoid Hungarian Notation
In
If you change the variable's type (e.g. to an
params keyword//The new Logger for the exe
static void WriteLog(string exception, int status, params string[] arguments)
{
string logFile = string.Format("D:\\TempLogs\\Grep-{0:yyyy-MM-dd_hh-mm-ss-tt}.log", DateTime.Now);
StreamWriter grepLog = new StreamWriter(logFile, true);
grepLog.WriteLine("Status: " + status.ToString() + " (0 for PASS, 1 for FAIL)");
grepLog.WriteLine("Exception: " + exception);
// TODO: Consider logging "no arguments" if that is the case.
for(int i=0; i<arguments.Length; i++)
{
grepLog.WriteLine("Argument " + i + ": " + arguments[i]);
}
grepLog.Close();
}Use an
enum for statusSince the status has 3 states, use an
enum for it. This will help you avoid mistakes.Why do you try-catch while logging?
If you don't see any particular possibility for the log to fail, then simply assume it and don't try-catch the logging.
If you want the logging to no matter what not break the application with exceptions, and you cannot fix those exceptions (why?), then I would suggest putting the try-catch inside the logging method.
Main - exception should catch, log, and return
I would have the exception log the error and return, instead of continuing normally.
Alternatively, I would catch, log, and re-throw:
try { ... } catch (Exception e) { WriteLog(...); throw; }Use camelCase for local variables
StreamWriter grepLog;Using a starting underscore (
_grepLog) is also common for private fields.Follow the de-facto conventions. (See e.g. https://stackoverflow.com/questions/2976627/on-c-sharp-naming-conventions-for-member-variables )
Avoid Hungarian Notation
In
string strGrepLogFileName, the part "str" brings nothing new.If you change the variable's type (e.g. to an
Uri) without changing its meaning, you should not have to rename the variable, and using Hungarian Notation you do.Code Snippets
//The new Logger for the exe
static void WriteLog(string exception, int status, params string[] arguments)
{
string logFile = string.Format("D:\\TempLogs\\Grep-{0:yyyy-MM-dd_hh-mm-ss-tt}.log", DateTime.Now);
StreamWriter grepLog = new StreamWriter(logFile, true);
grepLog.WriteLine("Status: " + status.ToString() + " (0 for PASS, 1 for FAIL)");
grepLog.WriteLine("Exception: " + exception);
// TODO: Consider logging "no arguments" if that is the case.
for(int i=0; i<arguments.Length; i++)
{
grepLog.WriteLine("Argument " + i + ": " + arguments[i]);
}
grepLog.Close();
}Context
StackExchange Code Review Q#18501, answer score: 10
Revisions (0)
No revisions yet.