Is it possible to do something like this in c#, where we are trying to call a different method as a part of a conditional statement that uses the "?" operator?
Sample Code
private bool myTenStepMethodVersion()
{
var stepPassed = false;
stepPassed = StepOne(reportDate);
stepPassed = (stepPassed) ? StepTwo() : RecordTimeAndLogFailure(timer,reportDate,"Step2");
stepPassed = (stepPassed) ? StepThree() : RecordTimeAndLogFailure(timer,reportDate,"Step3");
etc.
}
private bool StepOne(DateTime someDate)
{
//do stuff
return true;
}
private bool StepTwo()
{
//do stuff
return true;
}
private bool RecordTimeAndLogFailure(timer,someDate)
{
//write to a log the step where we failed, timer and date.
}
The idea was to avoid having 10 different if / then combos for a 10 step process.
This type of an approach kinda works until: a) step3 needs some data created by step 2? aka. I want to return something other than a bool? I guess we can create private global variables in the class and have each step check it? is there another way?
b) After we run RecordTimeAndLogFailure(), we need to actually just exit the entire method. How can we do that if we follow this type of an approach?
Overall, the goal is to make the code more readable. The current code has multiple if / then statements and over 60 lines in the main method
CodePudding user response:
Exceptions for error handling come to mind immediately:
class StepException : Exception {
public string Step { get; }
public StepException(string step) {
this.Step = step;
}
}
private bool myTenStepMethodVersion2()
{
try
{
stepOne(reportDate);
var data = stepTwo();
stepThree(data);
// …
return true;
} catch (StepException ex) {
RecordTimeAndLogFailure(timer, reportDate, ex.Step);
return false;
}
}
private void StepOne(DateTime reportDate)
{
if (error) { throw new StepException("Step2"); }
// do stuff …
}
private StepTwoResult StepTwo()
{
var result = new StepTwoResult();
// do stuff ...
if (error) { throw new StepException("Step2"); }
return result;
}
private RecordTimeAndLogFailure(timer,reportDate)
{
//write to serilog the step where we failed, timer and date.
}
CodePudding user response:
I would avoid Exceptions, they are costly in terms of performance and should not be used to drive the logic of the code.
Instead I would go for this:
var stepPassed = false;
string failurePoint = "Step1";
stepPassed = StepOne(reportDate);
failurePoint = "Step2";
if(stepPassed) stepPassed = StepTwo();
failurePoint = "Step3";
if(stepPassed) stepPassed = StepThree();
if(!stepPassed)
RecordTimeAndLogFailure(timer,reportDate,failurePoint);
The ifs are still there but are lot easier to read. And the logging happens just at the end with a single call. If you want, you can hide the failurePoint variable making it global to the class and setting it inside the single Steps, but I prefer to keep a local variable.
Of course if some of your steps should give back some internal value used in next steps you sould always use out parameters