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.










share|improve this question




















  • 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















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.










share|improve this question




















  • 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













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.










share|improve this question















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






share|improve this question















share|improve this question













share|improve this question




share|improve this question








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














  • 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















active

oldest

votes











Your Answer





StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");

StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});


}
});














draft saved

draft discarded


















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






























active

oldest

votes













active

oldest

votes









active

oldest

votes






active

oldest

votes
















draft saved

draft discarded




















































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.




draft saved


draft discarded














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





















































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







Popular posts from this blog

Сан-Квентин

8-я гвардейская общевойсковая армия

Алькесар