Simpe PHP Code Question

January 4, 2006 12:13 PM

How do I arrange this PHP code so that it does what it is supposed to do?

First of all, here is the code:

http://rafb.net/paste/results/QUVmtU10.html

Specifically, I'm concerned with lines 6-48, the IF/ELSE stuff. I don't know PHP but I can usually figure code like this with a little trial and error. Thus far I've failed with this piece though. At first it was just a bunch of IF statements with an ELSE at the end. Then I tried using a flag so that anytime the IF failed, the ELSE would be triggered. That didn't work either. Basically, I want the IF/ELSE statement to apply to each of the "IFs." Right now, it logically works with the "photos" line since that's the only one where the if is followed directly by the ELSE (I thought the flags would fix this but it didn't). I tried also putting the ELSE statement after each of the IFs but that seems to break things. How do I arrange the code so that it works with "bookmarks," "links" and "clippings"? Presently, when I click the photos link, it only shows photos. But for the other three, it'll show both the IF and the ELSE. I hope my question makes sense. Thanks for any help you provide. If needed, I can post a link to the blog where the code is presently in use (but not correctly implemented obviously).

Feel free to use Paste (); if you want to alter the code.

posted by panoptican to computers & internet (35 comments total)

You set the flag to false each time. So, if you've set the flag to true because "bookmarks" wasn't included, it's going to ignore that because you set it back to false again.

How about the following:

if(!empty($bookmarks)) include "bookmarks.php";
if(!empty($clippings)) include "clippings.php";
if(!empty($links)) include "links.php";
if(!empty($photos)) include "photos.php";

$flag = false;
if( !empty($bookmarks) || !empty($clippings) || !empty($links) ||
!empty($photos)) $flag == true;

...

posted by thanotopsis at 12:26 PM on January 4, 2006

i'm not certain if i quite understand what you are attempting to accomplish here, so i apologize in advance if my advice is off the mark. anyway:

the flags don't fix things here because they are reset after each if/else construct -- the only time $flag is significant is directly following if statement where you check if 'photos' was set among the HTTP GET vars.

you could restructure things using "if/elseif/else", assuming this conforms to what you want to happen: i.e.:

if (bookmarks are set) { /* this is all pseudo-code, by the way */
...
} elseif (links are set) {
...
} elseif (clippings are set) {
...
} elseif (photos are set) {
...
} else {
... do post stuff ...
}

secondly, your code on line 48 does not parse. the colons here cause a syntax error and should be removed. it looks like you mean to say:

while (have_posts()) {
the_post();
}

the "if (have_posts())" on line 48 is superfluous and can be removed entirely -- the first run-through of the while loop will check if have_posts() is true before it executes its code block in every iteration.
posted by moz at 12:27 PM on January 4, 2006

How about this
posted by holloway at 12:28 PM on January 4, 2006

It's impossible to tell what that code is even intended to be doing, what with all the pointless flags. Is the idea that if none of the ifs are true then it displays the posts? If so, just take out all but the first $flag=false.
posted by kindall at 12:29 PM on January 4, 2006

How about this

Your line 16 still is a syntactical combination of an if/then/else statement is a () ? : decision statement. You're going to want to fix that.
posted by thanotopsis at 12:32 PM on January 4, 2006

From your new example., I guess I misunderstood your code as well. From your new example, it appears that this code will only be executed with one query string argument.

In the new example, line 14 is going to prohibit your "default" tag in line 15 from being evaluated at all.
posted by thanotopsis at 12:35 PM on January 4, 2006

Yeah I just cut/n/paste that from the source.

How about this
posted by holloway at 12:36 PM on January 4, 2006

Why are you using different variables as flags?

What you want is:

<? php

//the incoming variable is called "$action"

switch($action){
case "bookmarks":
//include your bookmarks file
break;
case "clippings":
//include your clippings file
break;
case "photos":
//include your photos file
break;
default:
//in case action doesn't equal any of those things
break;

}

?>

posted by bshort at 12:38 PM on January 4, 2006

I mean it's hard to know what have_posts() and the_post() do. Presumably the_post() writes one thing but does it have an counter or something, so each time it's called it writes the next? And have_posts checks the counter?

It's hard to optimise that bit without knowing more.
posted by holloway at 12:39 PM on January 4, 2006

This is a WordPress template; the reason for the if have posts condition is that it's followed by an 'else echo sorry, no posts are available'. Basically this is a snippet from a larger file.
posted by Firas at 12:40 PM on January 4, 2006

Sorry I wasn't as clear as I could be. I'm a little in over my head with the php stuff. kindall, yes if none of the ifs are true, I would like the posts to show. It's probably just easiest that I link to the blog so you can see how it works now. SELF LINK. The links are in the top bar there.

thanotopsis, I tried your code but I ended up with the whole thing being blank where posts should have been. holloway your code is broken in some manner. I'm taking out the "extra" flags as per kindall's suggestion right now. Will let you know how it works.
posted by panoptican at 12:42 PM on January 4, 2006

Yeah I just cut/n/paste that from the source.

Oh, so this isn't your own source?

Your if/then/else is properly formatted now, but it doesn't dismiss the problem you're still going to have: The "break;" statement of line 14 kicks you out of the switch() block entirely, meaning you'll never get a chance to operate through that while() loop if you've included any of those files. Was that your intent (one or the other) or was it your intent to do both?
posted by thanotopsis at 12:42 PM on January 4, 2006

Change:

if( !empty($bookmarks) || !empty($clippings) || !empty($links) || !empty($photos)) $flag == true;

to

if( !empty($bookmarks) || !empty($clippings) || !empty($links) || !empty($photos)) $flag = true;
posted by Firas at 12:44 PM on January 4, 2006

If I read the have_posts() condition correctly, it's usage in the while() loop implies that it's providing a true/false indicator of some array being shifted out by calling the_post(). When the_post() shifts out the last item in the array, the next call to have_posts() will return false.
posted by thanotopsis at 12:46 PM on January 4, 2006

Ah, good catch, Firas, I wrote that rather quickly.
posted by thanotopsis at 12:47 PM on January 4, 2006

As Firas points out, this is a snippet from a larger file. Perhaps it would help if I posted it all.

Full code.

In the meantime, I'm trying out a bunch of these. Thanks so much all of you for the quick responses. I'll be back soon to let you know how it works out. Feel free to continue to offer suggestions though if you have any different ideas.
posted by panoptican at 12:49 PM on January 4, 2006

bshort may have it, if this will only be run through once and you might not have multiple conditions to check at the same time.

Are each of these values (bookmarks, links, clippings) actually holding a set of values? It looks to me like you're not understanding variable scoping. You're constantly resetting the flag variable between if statements without checking it; I suspect you're thinking of an "else if" conditional but that would still be overkill.

If so, I would replace it with this logic:

$flag = true;

if ( isset($_GET['bookmarks']) )
{
include('bookmarks.php');
$flag = false ;
}
if ( isset($_GET['links']) )
{
include('links.php');
$flag = false;
}

and so on.
posted by mikeh at 12:49 PM on January 4, 2006

Nevermind, it looks like you've gone with the switch statement, it's the better choice anyway.
posted by mikeh at 12:51 PM on January 4, 2006

Oh, so this isn't your own source?

That line is from the poster of the question. The array shift / array keys / switch block I wrote. Because they were blackbox functions with weird ass syntax I didn't want to mess with it.

The line 14 thing... panoptician's recent post says (paraphrased) "if nothing is included then show the posts" so I think that's what he wants. Maybe I'm reading him wrong?

My array_shift/array_keys technique assumes the page name is the first item in _GET. PHP makes _GET based on the url parameter order though, so this should be a safe assumption.
posted by holloway at 12:59 PM on January 4, 2006

thanotopsis, using your corrected code, I still get an empty document.

moz, I'm guessing your code doesn't work because you didn't have sufficient information. Also, I'm not even sure I implemented it correctly. It gives me an error.

holloway, your code gives me the following error (so apparently the switch statement conflicts with something later on in the code?):

Parse error: parse error, unexpected T_ENDWHILE in /home/.carroll/panoptican/panoptican.org/words/wp-content/themes/tiny/index.php on line 39

bshort/mikeh, I'm not sure I understand how to use that code. I'm fairly certain I arranged it wrong. Whatever the case, it gives me an error.

So in short, nothing worked but then, I'm not really sure what I'm doing. I mostly just copied and pasted what you all suggested. Perhaps I'm using your code correctly and perhaps I should just do something a little more simplistic given my lack of PHP knowledge.
posted by panoptican at 1:21 PM on January 4, 2006

mikeh, I just saw the code you suggested and tried it out but I end up with the original problem whereby only the photos link really works.
posted by panoptican at 1:26 PM on January 4, 2006

thanotopsis, using your corrected code, I still get an empty document.

Use halloway's corrected if..while block. Your block is badly formatted.
posted by thanotopsis at 1:32 PM on January 4, 2006

how about this?
posted by holloway at 1:57 PM on January 4, 2006

Oh, remove that "print 'nuttin';" line. That was debugging.
posted by holloway at 1:57 PM on January 4, 2006

Using your code holloway, I still get an error with the ENDWHILE later on in the script.

Parse error: parse error, unexpected T_ENDWHILE in /home/.carroll/panoptican/panoptican.org/words/wp-content/themes/tiny/index.php on line 41
posted by panoptican at 2:09 PM on January 4, 2006

Mmm. Keep the WordPress loop seperate from your navigation fiddling at the top or things will get bizarre.

Try this.
posted by Firas at 2:23 PM on January 4, 2006

Must be a problem elsewhere then because here's the standalone version which works for me.

The block of code you've now added is a fragment that supposed to end a While loop. You'll need to retrace your steps and find how the the missing start piece of that fragment. It should just be a while loop.

If it should be in the existing while loop, then insert the fragment there rather than under the new code.
posted by holloway at 2:30 PM on January 4, 2006

I don't much like the die() bit, but Firas's code is the best one yet :)
posted by holloway at 2:32 PM on January 4, 2006

I'm using your code right now Firas. However, it doesn't seem to choose one or the other. In other words, if you click on any of the links, it just throws the contents of the link on top of the posts. SELF LINK to see.

Also, in your code, you're missing a ?> in line 23, just in case anyone else is looking at this.
posted by panoptican at 2:35 PM on January 4, 2006

I have no idea what the $pageName = key($_GET); thing does. I figured I'll take a proper look at it rather than sending you round in circles—recoding now…

holloway: yeah, I understand the reluctance to use die(), but with a blog template it's far more important that the code be usable to a novice than that it be elegant.
posted by Firas at 2:41 PM on January 4, 2006

This is how I would do it.
posted by Firas at 2:47 PM on January 4, 2006

It sort of works. It doesn't like something on line 16.

Warning: main(.php): failed to open stream: No such file or directory in /home/.carroll/panoptican/panoptican.org/words/wp-content/themes/tiny/index.php on line 16

Warning: main(): Failed opening '.php' for inclusion (include_path='.:/usr/local/lib/php') in /home/.carroll/panoptican/panoptican.org/words/wp-content/themes/tiny/index.php on line 16

Also, thanks so much everyone for going beyond what I expected. I thought it was a simple question. It's probably been made more difficult by my lack of knowledge on PHP. Anyway, AskMeFi never ceases to amaze me.
posted by panoptican at 2:58 PM on January 4, 2006

Almost there :) Replace "include($navPage.'.php');" with "include($pageName.'.php');"
posted by Firas at 3:09 PM on January 4, 2006

And Firas wins. Hooray for Firas!
posted by panoptican at 3:14 PM on January 4, 2006

Heh, I'm just intimately familiar with the blog system at hand so the deck was quite stacked. Glad it worked out.
posted by Firas at 3:18 PM on January 4, 2006