Details
Major
Description
CRM/Report/Form/Event/ParticipantListing.php
if ($value = $row['civicrm_participant_event_id']) {
if ($value = $row['civicrm_event_event_type_id']) {
if ($value = $row['civicrm_participant_status_id']) {
if ($value = $row['civicrm_participant_role_id']) {
if ($value = $row['civicrm_participant_participant_fee_level']) {
if ($value = $row['civicrm_address_country_id']) {
if ($value = $row['civicrm_address_state_province_id']) {
if ($value = $row['civicrm_contact_employer_id']) {
Array declaration that contain text character variables or values without quotations:
array('localize' => TRUE));
Attachments
Activity
John,
Just curious to know the problems you notice.
The conditions in question i think would return the boolean interpretation of value assigned to $value.
Here are some examples that assignment inside condition is not wrong:
http://www.php.net/manual/en/control-structures.if.php
http://www.sitepoint.com/assignment-inside-a-condition
In any case if you looking to improve them, here is one example:
if ($value = CRM_Utils_Array::value('civicrm_participant_status_id',$row))
Personally i don't see any big improvement as key is always going to exist in $row, but still a better practice.
Another example:
if (CRM_Utils_Array::value('civicrm_participant_status_id',$row))
Please note this is all over the place in reports. Would be good to get a patch for all.
There is nothing "technically" wrong with assignments-in-conditionals. The problem with them (and the reason some IDEs flag them as errors) is with readability and maintainability. If you are in the habit of doing assignments in conditionals then a typo where you accidentally type = instead of == would be very difficult to debug. But if we are strict and don't allow them in our code, then there's no ambiguity and we know that any = symbol in a conditional is a typo and should be fixed.
so the goal is for JohnFF to submit a PR to clean up the code towards readability and maintanabiity and to move away from using assignment in conditional
John Kirk Can you point me to the patch / PR ?
Hi, I've logged pull requests for
https://github.com/civicrm/civicrm-core/pull/3441
https://github.com/civicrm/civicrm-core/pull/3442
https://github.com/civicrm/civicrm-core/pull/3495 < ParticipantListing.php
1) Commuted assigns in if statements into assigning, then checking, as otherwise fails code validators and breaches CiviCRM coding conventions.
2) Refactored repeated code found while repairing those errors into a separate reusable function that accepts parameters. I only did this where the code was identical. Where there was any distinction I didn't do this to prevent introducing bugs.
3) Was able to select more appropriate names for these variables than "value".
https://github.com/civicrm/civicrm-core/pull/3496 < Participant.php
4) Commuted assigns in if statements to assigns that are then checked in if statements.
5) Set appropriate names for these vars.
Nice readability improvements.
john:
can u please submit a PR for this and clean the code. The code is correct as is, but using an assignment as a conditional seems a bit cryptic