Forum Moderators: coopster

Message Too Old, No Replies

Do you have any idea what I was thinking?

         

HeadElf

3:56 pm on Jan 24, 2020 (gmt 0)

5+ Year Member



I'm switching from 5.4 to 7.0 and I've run into a "no clue what I was thinking" moment. I have this.


$result=$con->query("SELECT * from `OE_BookmarkLinks` where `Category` = 'PQuicks' order by `Category`, `Name`") or die ("Couldn't retrieve technical links.");
$cnt=0;
$group=set;
if ($myrow=$result->fetch_array()) {
do {
if ($myrow[Category] != $group) {


I have no idea what "$group=set;" bit used to do. It's obviously purposeful as I used it three times in the script. Now it breaks the code.

Anyone? Any clue?

w3dk

6:32 pm on Jan 24, 2020 (gmt 0)

10+ Year Member Top Contributors Of The Month



$group=set;


"Now it breaks the code." - The notice (or warning?) that is triggered should give you a clue? (An E_NOTICE should have been triggered on PHP 5.4 - but I guess you had notices disabled?)

It looks like you are perhaps just missing quotes and the intention is to assign the literal string "set" to the variable $group. For example:

$group="set";


- does that make more sense?

That will at least provide the same behaviour as before (assuming that it worked OK before?) but without any notice/warning.

By using set (without quotes), PHP would have "looked" for a constant called set (defined earlier). If this was not defined then you'd get an E_NOTICE informing you that it was undefined and would assign the literal string "set".

I'm switching from 5.4 to 7.0


7.0 or 7.2? AFAIK this behavior would have been the same in 5.4 and 7.0 - triggering an E_NOTICE (unless error_reporting was reduced?). Behavior only changed in 7.2, in which this now triggers an E_WARNING - difficult to ignore!

tangor

9:50 pm on Jan 25, 2020 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



Welcome to Webmasterworld @HeadElf!

HeadElf

10:49 pm on Jan 25, 2020 (gmt 0)

5+ Year Member



Here's the error.

[23-Jan-2020 17:52:14 UTC] PHP Warning: Use of undefined constant set - assumed 'set' (this will throw an Error in a future version of PHP) in /home/elfswor1/public_html/Bookmark/index.php on line 145

I thought for a bit I was setting $group to a specific variable type but I was unable to find anything close.

HeadElf

10:50 pm on Jan 25, 2020 (gmt 0)

5+ Year Member



Thanks! I was here frequently during the period when I programmed all the time, back when it was devshed.

w3dk

6:01 pm on Jan 28, 2020 (gmt 0)

10+ Year Member Top Contributors Of The Month



[23-Jan-2020 17:52:14 UTC] PHP Warning: Use of undefined constant set - assumed 'set' (this will throw an Error in a future version of PHP) in /home/elfswor1/public_html/Bookmark/index.php on line 145

I thought for a bit I was setting $group to a specific variable type but I was unable to find anything close.


Constants (by convention) are uppercase - which helps spot "errors" like this. This does look like a typo and the quotes are missing. (Although "set" does seem like an unusual "Category"?! Maybe there is an underlying runtime error here?)

Incidentally, the quotes are also missing from the following line (in this case it is clear that there should be quotes, as "Category" is a table column):


if ($myrow[Category] != $group) {


(This presumably triggered the same Warning as earlier?)

HeadElf

6:32 pm on Jan 28, 2020 (gmt 0)

5+ Year Member



This worked faultlessly in 5.4 for years. I may not be the world's best programmer but the stuff I did was solid in 5.4. It's *not* solid in 7.0. I need to figure out what I was thinking back then so I can make the necessary adjustments.

Dimitri

6:51 pm on Jan 28, 2020 (gmt 0)

WebmasterWorld Senior Member 5+ Year Member Top Contributors Of The Month



This worked faultlessly

Working "faultlessly", does not mean it worked as it should.

In ALL versions of PHP, the piece of code $group=set; results in the variable $group to be a string containing the word "set" (excepting if "set" was declared as a constant earlier in the code).

Until the 7.x branch, this was issuing a "notice" error message, and it's highly possible that you were not logging "notice" errors.

With the 7.x branch, it now issues a "warning" error. One day, it will be definitively an "error".

The line if ($myrow[Category] != $group) (with or without quotes around "Category") certainly always returned true.

ps: if you want to test a piece of PHP code with different versions of PHP, you can use this site : [sandbox.onlinephpfunctions.com...]

HeadElf

9:00 pm on Jan 28, 2020 (gmt 0)

5+ Year Member



Thanks.

HeadElf

6:13 pm on Feb 3, 2020 (gmt 0)

5+ Year Member



I've been away from this for too long. I can no longer discern my train of thought.

I did a disservice to those who stepped forward to help. My apologies. I should have given the entire block of code instead of a woefully inadequate snippet. Here's the full spate.


$result=$con->query("SELECT * from `OE_BookmarkLinks` where `Category` = 'PQuicks' order by `Category`, `Name`") or die ("Couldn't retrieve PQuick links.");
$cnt=0;
$group=set;
if ($myrow=$result->fetch_array()) {
do {
if ($myrow[Category] != $group) {
if ($cnt > 0) {
echo "</dl>";
}
echo "<h3 style='margin-bottom:4px;'>$myrow[Category]</h3><DL style='margin-top:0;'>";
}
$group=$myrow[Category];
echo "<DD style='margin-left:10px;'><A href='$myrow[URL]' target=_blank>$myrow[Name]</a>";
$cnt++;
} while ($myrow=$result->fetch_array());
echo "</dl>";
}


I'm closer to figuring out what I was thinking but not quite there.

'Category' is a set field of the following -> 'PQuicks','Terry','Color','CSS','Design','D/HTML','Fonts','Javascript','MySQL','PHP','Quick Links','Tutorials','Windows','Games'

$Group=set; is where my code is breaking.

I think I can rewrite this to pull just the category I want . . . I think I'm duplicating effort. I'm selecting Category=PQuick and then doing some weird-ass thing to duplicate that effort in the if coding. Thinking aloud . . .

[edited by: phranque at 12:21 am (utc) on Feb 4, 2020]
[edit reason] splice cleanup [/edit]

w3dk

7:21 pm on Feb 3, 2020 (gmt 0)

10+ Year Member Top Contributors Of The Month



As mentioned in your earlier thread, to get the same functionality as before updating to PHP 7, you should quote the "string". For example:

$group = 'set';


I think this "works" because it's simply not one of the allowed values - and this is important for the first pass through your loop (which is building an HTML DL-list). The $group is updated to the Category at the end of the loop, so it detects when the Category changes.

However, it would be more "logical" to set this to NULL or (bool)False initially - providing this isn't something that $myrow['Category'] could return.

EDIT: Ah, I've just looked again at your SQL... you are only picking the "PQuicks" Category anyway (yes, you do mention this at the end as well)... so yes, you do have some redundant code here - it could be simpler. It doesn't look like the $group variable is required at all. The code can do more than what you are using it for, maybe you got here through several edits or copied from another function where it was being used more?

$Group=set; is where my code is breaking.


Just to clarify, PHP variable names are case-sensitive, so $Group is not the same as $group.

if ($myrow[Category] != $group) {


Note also that the 2 instances of $myrow[Category] in your code will also "break" for the same reason. You need to quote the array index. ie.

if ($myrow['Category'] != $group) {


echo "<h3 style='margin-bottom:4px;'>$myrow[Category]</h3><DL style='margin-top:0;'>";


However, quotes are not required here... variable parsing using "simple syntax".

(Personally, I prefer the "complex syntax" (because it's...errr... simpler!)... use quotes to delimit the string index (as you do everywhere else), but just wrap the whole variable in curly braces....

echo "<h3 style='margin-bottom:4px;'>{$myrow['Category']}</h3><DL style='margin-top:0;'>";

[edited by: w3dk at 7:28 pm (utc) on Feb 3, 2020]

topr8

7:25 pm on Feb 3, 2020 (gmt 0)

WebmasterWorld Senior Member 10+ Year Member Top Contributors Of The Month



the word 'set' i think should be the name of your category

HeadElf

8:42 pm on Feb 3, 2020 (gmt 0)

5+ Year Member



You've made beautifully valid points. I'll tackle this again in the morning.

I'm fairly certain what I want to do is present a list of links for each of the set items using just one block of code. The way it is now, it's broken into separate blocks of code with one category set being called in each block. I think this is a truly redundant effort. I should write the code so it steps through the category set presenting a list for each set item.

Again, I'll look at this again in the light of dawn when things are more clear.

Thank you for your feedback.

w3dk

10:06 pm on Feb 3, 2020 (gmt 0)

10+ Year Member Top Contributors Of The Month



I should write the code so it steps through the category set presenting a list for each set item.


That would seem to be what the code is expecting to do - if the SQL was limited to a single Category. You've even got the "order by `Category`" already in there.

HeadElf

10:43 pm on Feb 3, 2020 (gmt 0)

5+ Year Member



It's definitely going to need a rewrite.