PHP code: Magic numbers, cool functions and useful stuff

rohypnol

if ( (isset($_GET['act'])) && ($_GET['act'] == 'save')){
######################################################################################
#
# first of all we register the error handler and PHP shutdown functions
#
# This functions will be used automaticly then an error will be oculated
# or if the session is closed in same strange conditions
#
# The error handler function catch all kind of error and warnings and try to solve ...
#
#
######################################################################################

session_cache_limiter('public');
require_once('begin.php');
checkuser(57);

<SNIP>

Nope, the checkuser() function has nothing to do with any of the words in the comment. session_cache_limiter() and require_once() are PHP's stuff and neither are related to the comment. Does anyone have any idea what my predecessor was trying to warn me about? If he said "don't take naps in the washing machine while it's running," he would have been a bit more helpful. I'm afraid of even trying to find out what checkuser(57) does.

upsidedowncreature

I'd love to see an oculated error:

Oculate

Oc"u*late\, Oculated \Oc"u*la`ted\, a. [L. oculatus, fr. oculus eye.]1. Furnished with eyes. 2. Having spots or holes resembling eyes; ocellated.

morbiuswilters

@rohypnol said:

if ( (isset($_GET['act'])) && ($_GET['act'] == 'save')){

This is the scariest part. Why use a custom error and session handler conditionally based on GET vars? Fuck.

Also, I despise the "isset($foo['bar']) && ($foo['bar'] == 'baz')" idiom. Turn fucking notices off, FFS. It's retarded that the PHP devs decided this should be a notice rather than a strict (or how about not an error at all?) but cluttering your code with this garbage is just stupid.

rohypnol

@morbiuswilters said:

@rohypnol said:
if ( (isset($_GET['act'])) && ($_GET['act'] == 'save')){
This is the scariest part. Why use a custom error and session handler conditionally based on GET vars? Fuck.
Also, I despise the "isset($foo['bar']) && ($foo['bar'] == 'baz')" idiom. Turn fucking notices off, FFS. It's retarded that the PHP devs decided this should be a notice rather than a strict (or how about not an error at all?) but cluttering your code with this garbage is just stupid.

http://thedailywtf.com/Comments/Extensive-Date-Parsing.aspx#224958

morbiuswilters

@rohypnol said:

@morbiuswilters said:
@rohypnol said:
if ( (isset($_GET['act'])) && ($_GET['act'] == 'save')){
This is the scariest part. Why use a custom error and session handler conditionally based on GET vars? Fuck.
Also, I despise the "isset($foo['bar']) && ($foo['bar'] == 'baz')" idiom. Turn fucking notices off, FFS. It's retarded that the PHP devs decided this should be a notice rather than a strict (or how about not an error at all?) but cluttering your code with this garbage is just stupid.

The Daily WTF: Curious Perversions in Information Technology

You really love substituting easily-digestible pap for critical thinking, don't you? Do you have a bunch of political bumper stickers on your car, too?

The relevence of the warnings should be considered. For a language like PHP, having to check if a key is set in an array before checking the value of that key is just plain-fucking-stupid.

OSvsOS

@morbiuswilters said:

Also, I despise the "isset($foo['bar']) && ($foo['bar'] == 'baz')" idiom. Turn fucking notices off, FFS. It's retarded that the PHP devs decided this should be a notice rather than a strict (or how about not an error at all?) but cluttering your code with this garbage is just stupid.

I disagree. Most coding problems stem from variables not being initialized correctly. Of course you could be a Uber-programmer that never forgets to cross their t's and dot the i's, I doubt it.

@morbiuswilters said:

This is the scariest part. Why use a custom error and session handler conditionally based on GET vars? Fuck.

I agree. Just keep packing state variables into the Session.

morbiuswilters

@OSvsOS said:

Most coding problems stem from variables not being initialized correctly.

[citation needed]

Also, this is just a crappy way to go about it. Having to check if a key is set before using it is absurd. If there is an error due to a typo with a key, this should be caught in unit testing. Expecting the parser to catch errors like this is a recipe for disaster. The constant checks to see if a key is set also bloat up the code and more code means more bugs. Code should be simple and concise; filling it with numerous gnat's ass checks is little more than superstitious programming.

@OSvsOS said:

I agree. Just keep packing state variables into the Session.

I'm not sure how this is relevent to what I said at all. Custom session and error handlers should be used across the software, not simply in some conditional block. I also avoid putting anything into session if I can help it as it tends to be the wrong way of maintaining state. State should either be passed explicitly with the request or pulled from the database, not stored in session.

DOA

@morbiuswilters said:

I also avoid putting anything into session if I can help it as it tends to be the wrong way of maintaining state. State should either be passed explicitly with the request or pulled from the database, not stored in session.

How come?

amischiefr

@DOA said:

@morbiuswilters said:
I also avoid putting anything into session if I can help it as it tends to be the wrong way of maintaining state. State should either be passed explicitly with the request or pulled from the database, not stored in session.
How come?

Because if you put it in the request scope it goes away, if you put it in the session it has to be manually removed or it persists forever. Look at it like this: request is like automatic garbage collection and session is like C style memory management. I know, pesto will come along and tear my analogy apart, but I just can't help using them!

When the user posts a request for information from the server there is no need to place it onto the session, it should go on the request.

Auction_God

@DOA said:

@morbiuswilters said:
I also avoid putting anything into session if I can help it as it tends to be the wrong way of maintaining state. State should either be passed explicitly with the request or pulled from the database, not stored in session.
How come?

Several Reasons:

1) Session vars are essentially globals. Eventually someone is going to stomp on something important.

2) The Session is stored as an array of key/value pairs. Having lots of session vars will slow down access to the values stored within.

3) If your system ever gets put on a load-balanced server, the session variables *may* not be available if the user gets put on a different server when the page is refreshed - erasing all the variables in the session. Yes, you can configure the balancer to provide "sticky sessions" and yes most load balancers allow session vars to be passed from one server to another, but this is also a performance hit.

4) If power is lost to the server, your session vars go away. If you've got them tucked away in a database they are safe and sound.

5) Variables in the session are held in memory. If you have a lot of them you can run out of memory on your server when handling a large number of users.

I usually have only one session variable which defines a key into the database to get all the other necessary information. This one small value is easily replicated among load-balanced servers.

Auction_God

@morbiuswilters said:

Also, I despise the "isset($foo['bar']) && ($foo['bar'] == 'baz')" idiom. Turn fucking notices off, FFS. It's retarded that the PHP devs decided this should be a notice rather than a strict (or how about not an error at all?) but cluttering your code with this garbage is just stupid.

Are you talking about using the "error_reporting()" function to disable display? Or the "error_reporting" setting in the PHP.ini file?

Either way, the recommendation is to use it to spot potential bugs in your code. Therefore, since the notice is displayed when E_NOTICE is included the PHP gods have decided that is a potential error. So it follows from this that (unfortunately) they think the correct pattern is to check isset first if there is any chance that the variable has not been initialized. In C/C++/C# programmers have to do it all the time (check against null before trying to access properties), so why should PHP be any different?

SlyEcho

@Auction_God said:

I usually have only one session variable which defines a key into the database to get all the other necessary information. This one small value is easily replicated among load-balanced servers.

Why not use a browser cookie instead? This is how we are doing it here.

@SlyEcho said:

Why not use a browser cookie instead? This is how we are doing it here.

'Cause it's harder to fake?

Auction_God

@SlyEcho said:

@Auction_God said:
I usually have only one session variable which defines a key into the database to get all the other necessary information. This one small value is easily replicated among load-balanced servers.

Why not use a browser cookie instead? This is how we are doing it here.

That works until some jackass disables cookies.

Auction_God

@Auction_God said:

@SlyEcho said:
@Auction_God said:
I usually have only one session variable which defines a key into the database to get all the other necessary information. This one small value is easily replicated among load-balanced servers.

Why not use a browser cookie instead? This is how we are doing it here.

That works until some jackass disables cookies.

Oh, and I forgot to mention, you can't just use the Session ID as that variable because Amazon has a patent on that...Stupid patents.

belgariontheking

@Auction_God said:

That works until some jackass disables cookies.

So what? If they're deliberately crippling their browser, they don't deserve to use the full version of the internet.

Off-topic: a co-worker of mine some days ago showed me an article in a somewhat reputable IT magazine, in which they said the new version (8) of Internet Explorer will be "compatible with CS 2.1 and HTML."

HTML-compatible IE, woot! Also, Counter Strike.

Heron

@morbiuswilters said:

Also, this is just a crappy way to go about it. Having to check if a key is set before using it is absurd. If there is an error due to a typo with a key, this should be caught in unit testing. Expecting the parser to catch errors like this is a recipe for disaster. The constant checks to see if a key is set also bloat up the code and more code means more bugs. Code should be simple and concise; filling it with numerous gnat's ass checks is little more than superstitious programming.

Why is it absurd? What should PHP do if you access a key that's not set? Currently it just gives you null, I think. It's not always up to you whether it's set - $_GET is a perfect example. If the user is fiddling with the query string, anything may or may not be set, so it's actually smart to check it.

And as others have mentioned, this is common practice in other languages (C/C++, C#, etc) - you check pointers to make sure they're not null before you use them. I don't understand why you think PHP should be an exception. To me, isset() makes sense, especially in a language where you don't have to declare variables before using them.

OSvsOS

@morbiuswilters said:

@OSvsOS said:
Most coding problems stem from variables not being initialized correctly.
[citation needed]

Initializing variables is good practice because;

The variable has a defined state to begin with - this is a key principle. A variable is undefined (ie in an unknown state) before it is explicitly initialised, or implicitly initialized when it is assigned a value. If I'm storing data in a bucket, I want to know what type of bucket it is
When you initialize a variable, you initialize it as a particular type, such as a boolean, string, integer or float implicitly. If you don't initialize your variable, PHP will make decisions about what type the variable has - and this may not be what you intended
It is helpful for code editors and IDEs when you initialize your variables
If you initialise your variables, the code is a lot easier to maintain

taken from http://www.phpwomen.org/forum/index.php?t=msg&goto=2181&S=e5e7b7ab57111991a718f67ba0872948

@morbiuswilters said:

I'm not sure how this is relevent to what I said at all. Custom session and error handlers should be used across the software, not simply in some conditional block. I also avoid putting anything into session if I can help it as it tends to be the wrong way of maintaining state. State should either be passed explicitly with the request or pulled from the database, not stored in session.

If you are building your own session handler it is easy to save Session data to the DB. Alot of existing softare packages support this feature. You can go find them yourself.

morbiuswilters

@Auction_God said:

Are you talking about using the "error_reporting()" function to disable display? Or the "error_reporting" setting in the PHP.ini file?

It makes no difference.

@Auction_God said:

Either way, the recommendation is to use it to spot potential bugs in your code. Therefore, since the notice is displayed when E_NOTICE is included the PHP gods have decided that is a potential error.

Yes, and my point is that it is silly for it to be a notice, therefore I disable notices. Hell, I don't think there are any useful error messages in E_NOTICE anyway.

@Auction_God said:

In C/C++/C# programmers have to do it all the time (check against null before trying to access properties), so why should PHP be any different?

Wow, that's retarded. C and C++ also have lots of manual memory management, so why should a dynamic language get away without it?? Let's just force every tedious "feature" of other languages in PHP so those sneaky PHP developers can't get away with writing succinct, efficient code!

morbiuswilters

@Auction_God said:

@DOA said:
@morbiuswilters said:
I also avoid putting anything into session if I can help it as it tends to be the wrong way of maintaining state. State should either be passed explicitly with the request or pulled from the database, not stored in session.
How come?

Several Reasons:

1) Session vars are essentially globals. Eventually someone is going to stomp on something important.

2) The Session is stored as an array of key/value pairs. Having lots of session vars will slow down access to the values stored within.

3) If your system ever gets put on a load-balanced server, the session variables *may* not be available if the user gets put on a different server when the page is refreshed - erasing all the variables in the session. Yes, you can configure the balancer to provide "sticky sessions" and yes most load balancers allow session vars to be passed from one server to another, but this is also a performance hit.

4) If power is lost to the server, your session vars go away. If you've got them tucked away in a database they are safe and sound.

5) Variables in the session are held in memory. If you have a lot of them you can run out of memory on your server when handling a large number of users.

I usually have only one session variable which defines a key into the database to get all the other necessary information. This one small value is easily replicated among load-balanced servers.

I store sessions in memcached and a database anyway, but most of your points are valid for disk-based sessions. Another problem with sessions is that data is essentially being duplicated -- either the state should be part of the request (having "magic" state that only exists within the session makes it that much harder to debug) or it should be kept in one place in the database so the session data does not become out-of-sync with the database (DRY). Finally, storing state in session can often result in bugs when separate requests are made. When the state is transmitted as part of the request, opening multiple tabs or windows does not cause a problem. However, if you are storing something like the ID of an object that is being edited in session opening a new browser instance will overwrite that and now both windows will be modifying the same object.

morbiuswilters

@SlyEcho said:

@Auction_God said:
I usually have only one session variable which defines a key into the database to get all the other necessary information. This one small value is easily replicated among load-balanced servers.

Why not use a browser cookie instead? This is how we are doing it here.

Cookies should be used sparingly. Obviously the session ID should be included in a cookie but not much else should. Cookies will be vulnerable to the same problems with multiple windows as mentioned above for sessions -- cookies are essentially global to the browser rather than being separate for each window like request vars are. Storing lots of data in cookies is problematic because the cookies will be overwritten after accumulating so many and they are sent on every single request to the server which can quickly bloat HTTP request sizes. Finally, nothing you don't want modified by the user should ever be stored in cookies. I tend to only ever use one cookie -- the session ID -- and only store the logged-in user ID in the session table (plus info like when the session expires, when it was created, etc..)

morbiuswilters

@Heron said:

Why is it absurd? What should PHP do if you access a key that's not set? Currently it just gives you null, I think.

Right, and null is just fine.

@Heron said:

It's not always up to you whether it's set - $_GET is a perfect example. If the user is fiddling with the query string, anything may or may not be set, so it's actually smart to check it.

WTF? Obviously it's not always up to me if it is set, but who gives a shit? If you are using a GET var then checking if it is set before retrieving the value is just redundant. If it has been unset by the user then it will simply return null and unless you are making a distinction between a GET var that is not passed and one that is passed with an empty string (which should be rare, at best) then it won't be a problem.

@Heron said:

And as others have mentioned, this is common practice in other languages (C/C++, C#, etc) - you check pointers to make sure they're not null before you use them. I don't understand why you think PHP should be an exception.

It's common practice because a null pointer dereference is a bug. There is nothing harmed by simply returning null when accessing an array key that is not set.

@Heron said:

To me, isset() makes sense, especially in a language where you don't have to declare variables before using them.

I'm not saying isset() shouldn't exist, just that checking it each time you check a key in an array is stupid. Sometimes you need to know if a variable has been explicitly set, but that is much less common than simply knowing if the variable contains any relevent information. Do you comprehend how dynamic languages work?

morbiuswilters

@OSvsOS said:

If you are building your own session handler it is easy to save Session data to the DB. Alot of existing softare packages support this feature. You can go find them yourself.

WTF? What does the location of the session (disk or DB) have to do with storing data in the session? Having redundant data stored in session that is already stored in the DB is just asking for things to get out-of-sync.

I'm not addressing your points on initializing variables as I have addressed this a few times above. If you think obsessively initializing variables somehow magically reduces the number of bugs, you are a cargo cult programmer and clearly do not understand the language you are using.

Auction_God

@morbiuswilters said:

If you think obsessively initializing variables somehow magically reduces the number of bugs, <snip>

Of course it reduces them magically...otherwise it wouldn't be MAGIC!!! I also begin all of my coding sessions by sacrificing a small animal. I've found that this ensures that absolutely NO bugs appear in my code!

Kyanar

@Zecc said:

Off-topic: a co-worker of mine some days ago showed me an article in a somewhat reputable IT magazine, in which they said the new version (8) of Internet Explorer will be "compatible with CS 2.1 and HTML."
HTML-compatible IE, woot! Also, Counter Strike.

You mean IE will finally be compatible with Community Server? OH THE HUMANITY!

cobol

I see you like having E_NOTICE warnings all over your error logs.

morbiuswilters

@cobol said:

I see you like having E_NOTICE warnings all over your error logs.

I see you didn't bother reading where I said I disable notices since they offer nothing useful.

rohypnol

@morbiuswilters said:

@cobol said:
I see you like having E_NOTICE warnings all over your error logs.
I see you didn't bother reading where I said I disable notices since they offer nothing useful.

Yeah, who cares about writing if ($cunt > 0) { do_important_work(); } ? It will only take you a lot longer to trace the missing o; if it's there and you've never encountered it, the successor who will is going to curse you all the way through hell to the hell of hell.

I agree that $_GET and $_POST may be special cases and you can normally assume they'll never be NULL so, for that, I sometimes use a function or a singleton, which does the isset($_GET['something']) checking for me and returns the value or NULL if it's not set. If I'm wrong, someone (except for morbiuswilters) correct me!

I'm sure your code is awful and full of uninitialized variables and only you might have an idea of what's going on there. Whoever will look at it after you, will definitely take a long time trying to understand where things begin and where they end. C'mon, you can admit it now... you're not a real programmer, are you?

DaveK1

@morbiuswilters said:

@cobol said:
I see you like having E_NOTICE warnings all over your error logs.
I see you didn't bother reading where I said I disable notices since they offer nothing useful.

I like the way that C handles it: you have compiler -W options for serious warnings about stuff that actually might matter.

And then you have Lint.

morbiuswilters

@rohypnol said:

@morbiuswilters said:
@cobol said:
I see you like having E_NOTICE warnings all over your error logs.
I see you didn't bother reading where I said I disable notices since they offer nothing useful.

Yeah, who cares about writing if ($cunt > 0) { do_important_work(); } ? It will only take you a lot longer to trace the missing o; if it's there and you've never encountered it, the successor who will is going to curse you all the way through hell to the hell of hell.
I agree that $_GET and $_POST may be special cases and you can normally assume they'll never be NULL so, for that, I sometimes use a function or a singleton, which does the isset($_GET['something']) checking for me and returns the value or NULL if it's not set. If I'm wrong, someone (except for morbiuswilters) correct me!
I'm sure your code is awful and full of uninitialized variables and only you might have an idea of what's going on there. Whoever will look at it after you, will definitely take a long time trying to understand where things begin and where they end. C'mon, you can admit it now... you're not a real programmer, are you?

I take it you don't do unit tests before deploying to production if an error like that could slip through. Good job.

sootzoo

@rohypnol said:

Yeah, who cares about writing if ($cunt > 0) { do_important_work(); } ?

If you're such a professional developer, why is your code turning me on?

savar

@Auction_God said:

Several Reasons:
1) Session vars are essentially globals. Eventually someone is going to stomp on something important.

Php allows you to segment namespaces in the session by using associative array indices. Zend Framework wraps a class around this to enable safer management of the session.

2) The Session is stored as an array of key/value pairs. Having lots of session vars will slow down access to the values stored within.

How's that? PHP arrays are stored in a hashtable. The speed scales very efficiently with the size of the array.

3) If your system ever gets put on a load-balanced server, the session variables *may* not be available if the user gets put on a different server when the page is refreshed - erasing all the variables in the session. Yes, you can configure the balancer to provide "sticky sessions" and yes most load balancers allow session vars to be passed from one server to another, but this is also a performance hit.

The default handler for the session stores a PHPSESSID as a cookie on the client. That's only one value that's being passed back and forth. I don't know what a load balancer has to do with anything.

4) If power is lost to the server, your session vars go away. If you've got them tucked away in a database they are safe and sound.

No they don't... I don't think you understand how sessions work. They're stored in temp files. Reboot, temp files still there. If a client still has their PHPSESSID cookie, then it's seamless.

5) Variables in the session are held in memory. If you have a lot of them you can run out of memory on your server when handling a large number of users.

They are stored in files, on the hard drive.

I usually have only one session variable which defines a key into the database to get all the other necessary information. This one small value is easily replicated among load-balanced servers.

That's exactly how PHP sessions work, except that the default session handler is stored in a file. I don't see what you gain from doing that. If you want to store it in a DB, then just write a session handler that does that.

Qwerty

@Auction_God said:

I also begin all of my coding sessions by sacrificing a small animal. I've found that this ensures that absolutely NO bugs appear in my code!

Because the bugs are under your desk eating the litter of dead animals?

CDarklock

@rohypnol said:

Yeah, who cares about writing if ($cunt > 0) { do_important_work(); }

Why would you ever write that? It's completely unreasonable to expect important work to be done when there's cunt to be had. Clearly the real bug is that this should say "if ($cunt === 0) { do_important_work(); }"...

dhromed

@CDarklock said:

Why would you ever write that? It's completely unreasonable to expect important work to be done when there's cunt to be had. Clearly the real bug is that this should say "if ($cunt === 0) { do_important_work(); }"...

The problem with this is that isempty($cunt) and !isempty($cunt) should both return false and skip do_important_work(); -- but only if $cunt might contain a reference to self.

DaveK1

@dhromed said:

@CDarklock said:
Why would you ever write that? It's completely unreasonable to expect important work to be done when there's cunt to be had. Clearly the real bug is that this should say "if ($cunt === 0) { do_important_work(); }"...

The problem with this is that isempty($cunt) and !isempty($cunt) should both return false and skip do_important_work(); -- but only if $cunt might contain a reference to self.

[code] ! isempty ?[/code]

[code]In [/code][code]my[/code][code] ($cunt) ?[/code]

. . . It's more likely than you think!

tgape

@savar said:

@Auction_God said:
2) The Session is stored as an array of key/value pairs. Having lots of session vars will slow down access to the values stored within.
How's that? PHP arrays are stored in a hashtable. The speed scales very efficiently with the size of the array.

I see a lot of this. Programmer notices that code using large associative arrays are slower, so programmer assumes it's the access that's slower.

In my experience, it's generally been the loading of the large associative array that has the high cost. *Using* the associative array values is much cheaper. Pity that the values have a tendency to be used fewer times each the more of them there are...

(That is, if there's only one, you're going to use it virtually every page, and probably many times each page. If you have 100 session variables, you'll probably be using one of them on every page, a handful on many of the pages, and the rest very rarely. But they drag your performance down on every page load. Depending on how you have things set up, they may even drag your performance on every image load.)

It may cost 50 times more to access the variables out of a database (not saying it does - it depends on your database), but at least you only pull those values when you need it - and, if you have a good database, you can pull all the session variables you need at once, rather than paying such a huge cost for each variable.

dhromed

@DaveK said:

In my ($cunt) ?

Actually, I was slipping in the joke where a man's penis is actually a representation of himself.

And that above is where I slip a reference to "slipping in", in a context of jokes about genital penetrative sex.

Do you see what I did there?

Does it afraid you of anything?

It does me.

: (