First of all, I edited your post for indentation... That was just bad... Your code should never be that heavily indented, HTML, PHP or otherwise. Indentation is first and foremost a tool for legibility. When it starts getting in the way of that, then something is wrong. Typically that means you are trying to do too much, or you could be doing something wrong. I would suggest looking at your HTML and figuring out what is causing all that indentation and clean it up or eliminate it. Now, you were asking about that PHP... I can't tell if you are saying you are actually having issues with this code, or if you just want help cleaning this up. Either way, I'm not just going to answer the questions you posed at the end, this is Code Review after all. I want to help you make this look better, so please read the entire post.
What is _e()
? What does it do? This is a horrible function name. Variables and functions should be descriptive, but not excessively so. Try to leave unnecessary words out of your variable and function names, such as the "the" in the_author()
. Another problem with these functions, two apparently echo HTML somewhere inside instead of returning a value that can be echoed like get_bloginfo()
does. Functions that return HTML are hard to reuse, functions that return values can be reused in many different ways. Another benefit to doing this is that the return values of those functions can be set to variables so that you only have to call them once. For instance, you are calling the_author()
twice, and if, as I suspect, this function accesses a database, you are adding unnecessary overhead to your website. Also, don't mix your quotations. Pick one.
Don't use globals! Here is your mantra, "Globals are bad. Eval is Evil." If you repeat that to yourself everyday and remember it, I guarantee you will be a better programmer for it. Not that you are using eval, but remember that anyways. Now, I'm assuming the reason you are setting $current_user
as a global is so that get_currentuserinfo()
can use it to set up the user class. Instead, set up your user class like normal, but then return it so that you can set its value to $current_user
directly.
Alright, I understand why you set $user_id
, I am even assuming that you set $key
so that it can easily be changed. But why set $single
? And why as a string? I would just remove that and declare TRUE
directly in the get_user_meta()
function.
Again, another global, but this time I have no idea where you are getting it from, and this is the biggest problem with globals. I can't help you here except to say, repeat your mantra. If you need to pass values from one function to another, use the function's parameters and return the results. If you need to pass values from one page to another, use the GET, POST, COOKIE, or SESSION arrays.
You should never use POST, GET, or any user supplied data directly. You are just asking for cross site scripting. Check out PHP's filter_input()
function if your PHP version is >= 5.2. Also, I am unsure, but can you actually use hyphens "-" in POST/GET data? I think its just like variables and such and this is considered an illegal character.
$update = filter_input( INPUT_POST, 'update-fav-sel-meta', FILTER_SANITIZE_STRING );
What is $search
? Where did it come from? I can't find it defined. PHP is probably throwing a warning here and then saying something along the lines, "Can't find $search, assuming SEARCH" and then proceeding as if you were comparing the string "search" to $fav_sellers
.
I'm not sure what this update_post_data()
function is that you are speaking of, but if you want to append data to a pre-existing array you would do so like this.
$array[] = $string;
Hope this helps, and sorry if I did not answer your initial question, its a bit vague.