I need some extra input on how to go about refactoring my PHP code. Its a big if else that looks ugly. But its implemented with MVC so I'm finding it very hard identifying what classes should be required.
Actually the whole idea is strange to me, because there still has to be an if somewhere. Either on a value leading to a method or on a value leading to a class. I understand that if done correctly there should hopefully be less if else statements.
Heres the context! I'm making a data dashboard for centres and centres have x amount of customers and those customers have y amount of users. Statistics include Messages, Conversions, Visitors, Dwell Time And LoyaltyMessages. I have queries that return results. This can be a specific demograph or all, a specific Customer or all.
It gets a bit more complex here. The charts (chart.js) can be different, bar, line, pie etc. When the chart is pie, the form of the data must change.
There can also be a comparison flag, this goes to another query but essentially the time range is doubled, the results split and stored by 'current' and 'previous'. All queries can do comparison.
After each query a HTML table of results is produced, this changes on whether its a pie chart or not, and if its a comparison no table is produced. I have a table builder service thats fairly generic, it changes based on a pie chart or not.
I have queries like:
getConversionRate($startDate, $endDate, $centre) // all customers, can compare, no demograph
getConversionRateCustomer($customer, $startDate, $endDate, $centre) // specific customer, no demograph
getConversionRateDemo($demograph, $startDate, $endDate, $centre) // all customers, specific dempograph
getConversionRateCustomerDemo($demograph, $customer, $startDate, $endDate, $centre) // specific customer, specific demograph
I've made a list of constants I can use, but that gave me more problems, because they all seem like they should be in separate classes:
ChartType:
const PIE = 0
const DOUGHNUT = 1
const BAR = 2
const LINE = 3
Customer:
const ALL = 0
const ONE = 1
const MULTI = 2
// OR
const ALL = 0
specific = id
Demograph:
const All = 0
const ONE = 1
const MULTI = 2
// OR
const ALL = 0
specific = string
Comparison:
const NOCOMPARE = 0
const COMPARE = 1
I have an OK understanding of interfaces and abstract classes (I know what they are but have never used them). Every idea I get quickly ends with similar if statements somewhere in the code
Heres an existing if statement:
if ($chartType == 'pie' || $chartType == 'doughnut') {
if ($customer == $this->allCustomers) { // if user wants stats including all tenants
if ($demograph == 'address') {
if ($comparison) {
$return = $this->getPostcodeCompare($cStartDate, $cEndDate, $centre, $demograph);
return Response()->json($return);
} else {
$result = $this->conversionRepository->getConversionRatePostcodePie($startDate, $endDate, $centre);
return Response()->json($this->buildConversionArray($result, $demograph, 'Area/District', $comparison));
}
}
} else { // if user wants stats of a specific tenant
if ($demograph == 'address') {
if ($comparison) {
$return = $this->getPostcodeCompare($cStartDate, $cEndDate, $centre, $demograph, $customer);
return Response()->json($return);
} else {
$result = $this->conversionRepository->getConversionRateTenantPostcodePie($customer, $startDate, $endDate, $centre);
return Response()->json($this->buildConversionArray($result, $demograph, 'Area/District', $comparison));
}
}
}
} else {
if ($customer == $this->allCustomers) { // if user wants stats including all tenants
if ($demograph == $this->noDemographs) { // if user wants stats including all tenants without demographs
$result = $this->conversionRepository->getConversionRate($startDate, $endDate, $centre);
return Response()->json($this->buildConversionArray($result, $demograph, 'Dates', $comparison));
} else {
$result = $this->conversionRepository->getConversionRateDemo($demograph, $startDate, $endDate, $centre);
return Response()->json($this->buildConversionArrayDemo($result, $demograph, $comparison));
}
} else { // if user wants stats of a specific tenant
if ($demograph == $this->noDemographs) { // if user wants stats of a specific tenant without demographs
$result = $this->conversionRepository->getConversionRateTenant($customer, $startDate, $endDate, $centre);
return Response()->json($this->buildConversionArray($result, $demograph, 'Dates', $comparison));
} else {
$result = $this->conversionRepository->getConversionRateTenantDemo($demograph, $customer, $startDate, $endDate, $centre);
return Response()->json($this->buildConversionArrayDemo($result, $demograph, $comparison));
}
}
}
P.S. Sorry for some bad naming, Customers where once called Tenants, and the functions still include Tenant as the name.
Aucun commentaire:
Enregistrer un commentaire