PHP code to return JSON data for various kinds of charts
up vote
-1
down vote
favorite
I need some input on how to go about refactoring my PHP code. It's 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.
php object-oriented json data-visualization
add a comment |
up vote
-1
down vote
favorite
I need some input on how to go about refactoring my PHP code. It's 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.
php object-oriented json data-visualization
2
The title of your question should reflect what your code does, not what you think should be done.
– FreezePhoenix
Nov 26 at 13:29
The more code you can show, the better — preferably at least the whole class. You've left us guessing at what other code exists in your application, and all we have is hints that other methods exist.
– 200_success
Nov 26 at 17:12
add a comment |
up vote
-1
down vote
favorite
up vote
-1
down vote
favorite
I need some input on how to go about refactoring my PHP code. It's 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.
php object-oriented json data-visualization
I need some input on how to go about refactoring my PHP code. It's 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.
php object-oriented json data-visualization
php object-oriented json data-visualization
edited Nov 26 at 17:10
200_success
127k15149412
127k15149412
asked Nov 26 at 12:56
Daniel Cull
1
1
2
The title of your question should reflect what your code does, not what you think should be done.
– FreezePhoenix
Nov 26 at 13:29
The more code you can show, the better — preferably at least the whole class. You've left us guessing at what other code exists in your application, and all we have is hints that other methods exist.
– 200_success
Nov 26 at 17:12
add a comment |
2
The title of your question should reflect what your code does, not what you think should be done.
– FreezePhoenix
Nov 26 at 13:29
The more code you can show, the better — preferably at least the whole class. You've left us guessing at what other code exists in your application, and all we have is hints that other methods exist.
– 200_success
Nov 26 at 17:12
2
2
The title of your question should reflect what your code does, not what you think should be done.
– FreezePhoenix
Nov 26 at 13:29
The title of your question should reflect what your code does, not what you think should be done.
– FreezePhoenix
Nov 26 at 13:29
The more code you can show, the better — preferably at least the whole class. You've left us guessing at what other code exists in your application, and all we have is hints that other methods exist.
– 200_success
Nov 26 at 17:12
The more code you can show, the better — preferably at least the whole class. You've left us guessing at what other code exists in your application, and all we have is hints that other methods exist.
– 200_success
Nov 26 at 17:12
add a comment |
active
oldest
votes
active
oldest
votes
active
oldest
votes
active
oldest
votes
active
oldest
votes
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f208443%2fphp-code-to-return-json-data-for-various-kinds-of-charts%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
2
The title of your question should reflect what your code does, not what you think should be done.
– FreezePhoenix
Nov 26 at 13:29
The more code you can show, the better — preferably at least the whole class. You've left us guessing at what other code exists in your application, and all we have is hints that other methods exist.
– 200_success
Nov 26 at 17:12