OpenTechStrategies / lisc-ttm

LISC TTM code. See https://ttm.lisc-chicago.org/.
GNU Affero General Public License v3.0
1 stars 4 forks source link

Is $question a string or something else, in various Bickerdike files? #45

Open kfogel opened 10 years ago

kfogel commented 10 years ago

On the issue #19 branch, I'm about to make a commit that in these files changes $question to $question_sqlsafe:

In each of those files, the last use of $question (well, now it's $question_sqlsafe) is an expression like this:

${$question_sqlsafe.$type.$i}

What's up with that? I thought $question_sqlsafe was a string... Does every PHP variable have an implicit field named "$type", or... Maybe I just need to look at the code more carefully?

Right now I'm in the middle of other issue #19 stuff. Filing this here so we can come back to it.

cecilia-donnelly commented 10 years ago

In this compound variable (or variable variable, according to the manual ), $type is drawn from the $type_array (see line 308 in participant_surveys_youth.php) so is equal to "parent," "adult," "youth," or "all" and $i is the loop counter variable that determines whether we are looking at pre, mid, or post surveys (e.g. $i=1 means pre-surveys).
The idea behind creating ${$question.$type.$i} was that I could save results for question 1, youth, pre-surveys and get them later based on a set variable name that was created inside a loop but was different for each result. That is, each result was saved in a variable with a sensible name (something like $question_1youth1). I think that use was deprecated when the chart creation and display were themselves included in the loop of questions - that is, there was no longer a need to save the result string for later use in the jqplot creation. All results are now put in the $script_str variable for use in the jqplot setup. $script_str is reset on each journey through the loop, so there's no longer a need for a specific variable for each result.

tl;dr: Yes, $question is a string.

kfogel commented 10 years ago

Cecilia Donnelly notifications@github.com writes:

The idea behind creating ${$question.$type.$i} was that I could save results for question 1, youth, pre-surveys and get them later based on a set variable name that was created inside a loop but was different for each result. That is, each result was saved in a variable with a sensible name (something like $question_1youth1). I think that use was deprecated when the chart creation and display were themselves included in the loop of questions - that is, there was no longer a need to save the result string for later use in the jqplot creation. All results are now put in the $script_str variable for use in the jqplot setup. $script_str is reset on each journey through the loop, so there's no longer a need for a specific variable for each result.

tl;dr: Yes, $question is a string.

Thank you :-).

So, is that line old code that we should remove now?

cecilia-donnelly commented 10 years ago

I think we're safe to remove that line (the ${$question.$type.$i}=$script_str; line) in: participants_surveys_youth.php participant_surveys_total.php participant_surveys_parents.php participant_surveys_individual_total.php participant_surveys_adults.php

And why yes, those probably all should be one file. Those are all the places it seems to appear.

On Tue, Jul 22, 2014 at 3:00 PM, Karl Fogel notifications@github.com wrote:

Cecilia Donnelly notifications@github.com writes:

The idea behind creating ${$question.$type.$i} was that I could save results for question 1, youth, pre-surveys and get them later based on a set variable name that was created inside a loop but was different for each result. That is, each result was saved in a variable with a sensible name (something like $question_1youth1). I think that use was deprecated when the chart creation and display were themselves included in the loop of questions - that is, there was no longer a need to save the result string for later use in the jqplot creation. All results are now put in the $script_str variable for use in the jqplot setup. $script_str is reset on each journey through the loop, so there's no longer a need for a specific variable for each result.

tl;dr: Yes, $question is a string.

Thank you :-).

So, is that line old code that we should remove now?

— Reply to this email directly or view it on GitHub https://github.com/OpenTechStrategies/lisc-ttm/issues/45#issuecomment-49792385 .

cecilia-donnelly commented 10 years ago

While investigating this issue, I learned that the Bickerdike pie charts are broken on development and production. See /bickerdike/data/participant_surveys_total.php for example. The branch Issue-45-cleanup was originally intended only for deleting the lines referenced in the previous comment, but now will probably have to look into this broader issue.

cecilia-donnelly commented 10 years ago

Documented pie chart problem in Issue #54.

cecilia-donnelly commented 10 years ago

This is ready to merge to master, since deleting the old lines of code did not cause the Issue #54 bug.