patterncsharpMinor
Calculating daily project drilling hours
Viewed 0 times
drillinghourscalculatingprojectdaily
Problem
I've got this method, and it's quite a big method. I look at it, and I feel... dirty. I feel that it's doing too much, and that there might be a better way to accomplish what I'm trying to.
This method returns 1 of 2 items: Either Daily Drilling Hours, or Total Project Drilling Hours. This was originally two methods, but I combined them because a lot of the code was being duplicated, and it made sense at the time (about 6 months ago). But now, looking at it, I'd like to improve this. Is there a better way?
Yes, I know about parameterized queries. I've already incorporated those in the final query, which would look something like this:
```
que
///
/// Calculates daily project drilling hours
///
/// Bool: True for daily, false for total project
/// Bool: True for specific time frame, false for whole day
/// Double: Drilling hours for time frame
public double CalculateDrillingHours(bool daily = false, bool timeFrame = false)
{
Logger.Info("Calculating Drilling Hours");
var query = "SELECT timestamp FROM myDatabase WHERE measured_dist = bit_loc AND rop > 0";
query = BuildQuery(query, daily, timeFrame);
var tempList = ExecuteQuery(query);
// The user needs to know if there was no data for the time frame chosen
var noData = new Exception("There was no data for the selected time frame. Please select another. (Drilling Hours)");
if (tempList.Count < 1)
{
throw noData;
}
if(!daily)
{
TotalProjectDrillingHours = Convert.ToDouble(Math.Round(TimeCalculations(ConvertStringListToDateTimeList(tempList)).TotalHours, 2, MidpointRounding.AwayFromZero));
Logger.Info("Total Drilling Hours calculated");
return TotalProjectDrillingHours;
}
DailyDrillingHours = Convert.ToDouble(Math.Round(TimeCalculations(ConvertStringListToDateTimeList(tempList)).TotalHours, 2, MidpointRounding.AwayFromZero));
Logger.Info("Daily Drilling Hours calculated");
return DailyDrillingHours;
}This method returns 1 of 2 items: Either Daily Drilling Hours, or Total Project Drilling Hours. This was originally two methods, but I combined them because a lot of the code was being duplicated, and it made sense at the time (about 6 months ago). But now, looking at it, I'd like to improve this. Is there a better way?
Yes, I know about parameterized queries. I've already incorporated those in the final query, which would look something like this:
```
que
Solution
I would consider first splitting the method into smaller methods as other people have mentioned. The only suggestion for now is I might suggest changing your bool daily to a enumeration. This in theory means it might be easier to add a Hourly,Monthly etc reporting option. Although this is only slightly better in my opinion and I'm sure there are better approaches (maybe using interfaces?).
Something like this as an exercise in splitting things out into various methods keeping most of your code but just re-arranging it slightly.
Something like this as an exercise in splitting things out into various methods keeping most of your code but just re-arranging it slightly.
public double CalculateDrillingHours(DrillingPeriod period = DrillingPeriod.Daily, bool timeFrame = false)
{
Logger.Info("Calculating Drilling Hours");
var tempList = GetDrillingTimes(period, timeFrame);
var drillingHours = CalculateDrillingHours(tempList);
SetDrillingTimes(period, drillingHours);
return drillingHours;
}
private void SetDrillingTimes(DrillingPeriod period, double drillingHours)
{
switch (period)
{
case DrillingPeriod.Daily:
Logger.Info("Total Drilling Hours calculated");
DailyDrillingHours = drillingHours;
break;
case DrillingPeriod.Total:
Logger.Info("Daily Drilling Hours calculated");
TotalProjectDrillingHours = drillingHours;
break;
default:
throw new NotImplementedException("Drilling period not supported");
}
}
private List GetDrillingTimes(DrillingPeriod period, bool timeFrame)
{
var query = "SELECT timestamp FROM myDatabase WHERE measured_dist = bit_loc AND rop > 0";
var daily = period == DrillingPeriod.Daily;
query = BuildQuery(query, daily, timeFrame);
var tempList = ExecuteQuery(query);
if (tempList.Any())
{
return tempList;
}
// I would look at possibly using a custom exception here i.e. NoDataException
throw new NoDataException("There was no data for the selected time frame. Please select another. (Drilling Hours)");
}
private double CalculateDrillingHours(List projectHours)
{
return Convert.ToDouble(Math.Round(TimeCalculations(ConvertStringListToDateTimeList(projectHours)).TotalHours, 2, MidpointRounding.AwayFromZero));
}Code Snippets
public double CalculateDrillingHours(DrillingPeriod period = DrillingPeriod.Daily, bool timeFrame = false)
{
Logger.Info("Calculating Drilling Hours");
var tempList = GetDrillingTimes(period, timeFrame);
var drillingHours = CalculateDrillingHours(tempList);
SetDrillingTimes(period, drillingHours);
return drillingHours;
}
private void SetDrillingTimes(DrillingPeriod period, double drillingHours)
{
switch (period)
{
case DrillingPeriod.Daily:
Logger.Info("Total Drilling Hours calculated");
DailyDrillingHours = drillingHours;
break;
case DrillingPeriod.Total:
Logger.Info("Daily Drilling Hours calculated");
TotalProjectDrillingHours = drillingHours;
break;
default:
throw new NotImplementedException("Drilling period not supported");
}
}
private List<DateTime> GetDrillingTimes(DrillingPeriod period, bool timeFrame)
{
var query = "SELECT timestamp FROM myDatabase WHERE measured_dist = bit_loc AND rop > 0";
var daily = period == DrillingPeriod.Daily;
query = BuildQuery(query, daily, timeFrame);
var tempList = ExecuteQuery(query);
if (tempList.Any())
{
return tempList;
}
// I would look at possibly using a custom exception here i.e. NoDataException
throw new NoDataException("There was no data for the selected time frame. Please select another. (Drilling Hours)");
}
private double CalculateDrillingHours(List<DateTime> projectHours)
{
return Convert.ToDouble(Math.Round(TimeCalculations(ConvertStringListToDateTimeList(projectHours)).TotalHours, 2, MidpointRounding.AwayFromZero));
}Context
StackExchange Code Review Q#25155, answer score: 5
Revisions (0)
No revisions yet.