Still however Legacy Code evokes a vision in me that it is code that is ugly, old, runs on mainframes, and is probably 3000 lines long, uses globals and questionable code practices.
This is a bad way to look at it. You are judging the past by present day standards. By that logic, tomorrow you'll be looking down on the standards of today that you are implicitly endorsing by this statement. You need to put yourself in their shoes, with what they had to work with at the time. "Best Practice" is often subjective and highly dependent on current state of technology, current industry standards, current laws, etc.. These things and many other factors change over time and that affects what is considered "Best Practice". Also, I would point out that most of your "pretty 1-liners" you are almost certainly alluding to are really wrappers for frameworks built upon frameworks that require a lot more coding under the hood to make it possible, so that "3000 lines long" statement is especially laughable.
And even if we can all agree that a certain piece of code is bad even by proper standards of the time.. you still have to consider other factors, e.g. what were the internal resources/policies behind it? For example, time and time again I see devs of clients publishing bad code and that was a direct result of their hands being tied because of "office politics." Some people choose to sit on a pedestal and claim they'd never compromise for things like that. Others decide that hey, they gave their 2 cents, that's on the boss, sticking to principles don't pay the bills. And chances are, these sort of factors will never be fully known to the public.
I guess my overall point here is, don't be so quick to judge a piece of code unless you are judging it within its context. That doesn't mean "leave it be" or "it's okay, don't fix it". If it's not satisfactory to current standards, do something about it. Just save the pretentiousness and snobbery for something else.
I echo maxxd. To me, any versioned code that is not the most recent (production) version is considered legacy. Moving from 1.0 to 2.0 could have involved adding new features and nothing in 1.0 version actually changed, so why should 1.0 be considered ugly or bad code? New versions aren't always about fixing bad, broken or outdated code; it's also about adding new stuff, changes due to "politics" or policies, old or new technology, etc..
notice how the space got encoded to a +, but the + got encoded to the encoded value of %2B. You want the generated token to look like the latter, so that when a visitor clicks on a link, it will decode %2B to a literal + instead of decode + to a space.
Also wanted to mention that I see in your original code you are using intval. FYI ctype_digit expects a string argument, so if you are converting it to an integer type then you are going to get unexpected results from ctype_digit. So to be clear, do not cast/convert the value as an integer before using ctype_digit. Posted variables should always be a string type, so you don't need to do anything special before using ctype_digit, but if you really want to be explicit, type cast to string:
// this should be okay..
if ( ctype_digit( $_POST['p1'] ) )
// ..but if you want to be more explicit
if ( ctype_digit( (string) $_POST['p1'] ) )
To be clear, around here "help" means "I'm doing this myself and I got stuck on this specific thing so please help me understand what I'm doing wrong".
It does NOT mean "I need this and I don't know how to do it so can someone please do it for me."
If you want the latter, I suggest you offer more than "respect".
On a sidenote.. Have you actually tried to sit down to translate it? I'm earnestly asking. My python skills are noob at best as well, but looking at that script..I'm pretty confident I could translate that to php easy enough.
After all this time I'm still getting the impression that $i doesn't contain what you expect. So once again I tell you to echo out $enquery to see what your query string actually shows. You said you did that and it didn't echo anything out, which is kind of impossible, unless it was visually hidden because of the rest of the html layout on your page. I then asked you to ctrl+f for it in viewsource or output a js console log or alert and... near as I can tell, you just ignored me.
Look, this is really simple. If it works if you hardcode a number, but doesn't work when you use $i then obviously$i doesn't contain what you expect. Either it's not getting a value, or it's getting a bad value, or maybe it's getting an id that just isn't in your table.
Look at the trigger error message, where you echo out $enquery.
Notice: SELECT * FROM news WHERE newsid= - has encountered an error at: ......
That red stuff right there. That's your query string your sending to the db. Notice how there is no number after newsid? So, $i is not getting a value. $i is defined as an argument passed to your editnews() function. You said way back in your original post:
This code displays on the 'Edit' page using the following above the DOC type:
So, assuming this code is really there and in the correct order/scope, are you including an "i" parameter on your page when you view it? e.g. mypage.php?i=123. I.. I feel like this too is something that was mentioned already.
I understand that the missing bracket was a typo. But when you say something works, then no, you don't get a free pass on expecting people to treat it as pseudo-code. Saying something works implies that you actually tested it and it should work. Can someone easily pick that error out and fix it? Sure. But I was nonetheless being pedantic because you were being an asshat via PM.
But that missing bracket isn't really the point. The point is that you don't understand what your code is doing and yet you are arguing that it works for some unknown reason, when it fact it doesn't.
"." does not allow you to put more instructions between an && when testing the TRUE of a statement. That is not what is happening at all. A dot is a concatenation operator; it joins strings together. Then you have several variable assignments going on which isn't going to work as you expect because of operator precedence. This is where things get really confusing, and honestly I don't fault you at all for not understanding wtf is going on.. it took me a good while to sort it out myself. What I do fault you is for not properly testing the code and passing it off saying it works, despite admitting you don't know what it's doing. Even if you don't know what's going on, you could have easily tested the code and seen that it doesn't work as expected, by doing exactly what I did in my last post.
So here is what is happening.
First, isset($_POST['str']) is evaluated, and (presumably) evaluates as true (which it does in my example where I just hardcode it to a value instead of receive a form input).
What really happens is everything on the right side of that first equal sign is evaluated and assigned to $str and if that overall value works out to a truthy value, then overall we have an equivalent of if (true && true). Why does it work out this way? The key is the 2nd example note in the Operator Precedence entry that states even though the assignment operator has lower precedence than most other stuff, you can still use it within a condition to make the condition evaluate the value assigned to the variable. This practically flips order of operations, making the equal sign one of the highest in precedence.
So, let's examine whether or not we get a truthy value from the rest of that stuff:
$_POST['str'] will have an actual value, whatever the user entered in the form. The same value evaluated in the first isset call. In my example I used a long string "aaa...". For this example I'm just going to assume it's "foobar" for the sake of space. So, so far we have "foobar" as the value being assigned to $str. At this point in time, it really doesn't matter what comes next. Even if everything else returns false or some other value, overall you have a truthy value. But let's be thorough and finish this exercise so that you may fully understand what is going on.
Next, we have a concatenation operator, which means we're going to start gluing shit to the end of "foobar" to make a longer string.
$length=strlen($str) is same concept as before.. we're going to evaluate strlen($str). Which is null because at this point, $str doesn't actually exist yet, and trying to pass nothing to strlen gives you null, and a Notice. So, if you had error reporting on, this would have been your first clue that something is amiss; you should have gotten something like this:
Notice: Undefined variable: str in test.php on line 8
This is a Notice level error. You claim that you have all error reporting turned on except strict. So you should have seen this. Which means your error reporting settings aren't what you think it is, or else you just lied on that count. Anyways.. since this returns false, we're still sitting with "foobar" as what's ultimately going to be assigned to $str.
So now we have $length=null.$length>=15. Firstly, we're actually still assigning things to $length at this point, since you have another concatenation operator between this and the last thing. So $length isn't defined at this point, so you should have received a 2nd notice:
Notice: Undefined variable: length in test.php on line 8
So null.undefined is undefined, and then undefined>=15 is false, so overall, $length=false. Then that gets concatenated onto "foobar", so overall we now have...
...which effectively makes nothing get concatenated to it and thus we effectively have $str="foobar" as the 2nd expression being evaluated in the case. And since "foobar" is a truthy value, the overall condition is true.
if (isset($_POST['str']) && $str=$_POST['str'] . $length=strlen($str) . $length>=15)
if (isset('foobar') && $str=$_POST['str'] . $length=strlen($str) . $length>=15)
if (true && $str=$_POST['str'] . $length=strlen($str) . $length>=15)
if (true && $str='foobar' . $length=strlen($str) . $length>=15)
if (true && $str='foobar' . $length=strlen(undefined) . $length>=15)
if (true && $str='foobar' . $length=null . $length>=15)
if (true && $str='foobar' . $length=null . undefined>=15)
if (true && $str='foobar' . $length=undefined>=15)
if (true && $str='foobar' . $length=false)
if (true && $str='foobar' . false)
if (true && 'foobar')
if (true && true)
Now, I hope you learned something here today. Firstly, don't just pawn something off as an answer when you don't understand what it's doing. Second, don't be a jackass about it, especially when you admit you don't understand what it's doing. But thirdly more important, I hope you now more fully understand the code and why it's wrong.