Functions should not rely on user input data

Functions should be given as much information as they need directly without relying too much on other data sources (such as $_GET/POST), with some exceptions.

1 - Does this mean for every function that expects POSTed or GETed values, it is necessary to set default function arguments values as well because otherwise PHP shows Warning missing argument in case the user won't pass any data?

I feel like I shouldn't weigh in because you're quoting something I said, but I will anyways.

Imagine the log_in function is a controller. It is, basically: it takes input from somewhere (the model) and probably won't output anything (the view).
As such it should stay separate from the model: it doesn't really care where the data came from so long as it has an email address and a password. Did it come from $_POST? Probably as that's the primary use case, but it could be somewhere else. By assuming it's always $_POST you're tightly coupling the function to the input and if something changes in the future then you have to rewrite that, and not just the function but anywhere the function is called.

So do it right to begin with: whatever is calling the function knows where the data is, so let it pass the data in.

Which doesn't actually address your questions.

1. Default values only affect the literal invocation of the function. If the calling code does not do due diligence and calls

PHP Code:

log_in($_POST["email"], $_POST["password"]);

without checking that the values are present in $_POST, $email and $password will be null because that's the value of $_POST["email"] and $_POST["password"]. The only way to trigger default values for parameters is to not pass anything at all as an argument:

PHP Code:

log_in()

Now, does it make sense to call the function that way? No. Logging in requires an email and a password and not giving either is ridiculous. By providing defaults you imply that either is optional when they are both necessary.

2. Default arguments aside, it is not harmful to check that $email and $password have values. Strictly speaking the code calling log_in should verify that there are email and password values, but log_in should itself verify the values.

There's one issue though: how do you differentiate between missing values and an invalid login (which would return false as well)? 10:1 odds that Jacques will tell you to use exceptions and I actually agree with that: they'd allow you to know exactly why this function failed.

PHP Code:

function log_in($email, $password) {
if ($email == "" || $password == "") {
// the "domain" of this function is non-empty emails and passwords
// (if you remember from math class, the domain of a function is its input
// and the range is its output)
// violating that precondition results in a DomainException
throw new DomainException("Email or password is empty"); // provided by SPL
}

You can actually go even more detailed for the different error conditions:
- Email or password is empty
- Email is not an email- Email does not exist Your function may do this but do not reveal this information to the user - if the email doesn't exist then it should be the same message you give if the password is incorrect
- Email and password do not match a user, aka incorrect password
in which case you should use different exceptions, such as DomainException, InvalidArgumentException, "NoSuchUserException", and "InvalidCredentialsException" (the last two being your own exception classes), so you can distinguish between the different types of errors.

Before replying to your comments I should check these with you. Please tell me what is wrong with each item below:

try, catch.

So many different tutorials on this! A lot of different points of views and confusion between different posters and those who reply to their comments, that kinda thing that makes Jaques1 angry! Ok please lets clarify:

try catch is for error handling in special situations. In order for the code to handle the errors with crashing.

How it works:

I donnot how to say it but "try" is always called. Like echo functionName(); So whenever is inside try gets executed!
Somewhere further there will be a thrwo new Exception with a message inside of it. This does not neccessarily get thrown.
Try has to be followed by a catch to show the error in case there is one. Basically:

Yes: what's in the try block is always executed until an exception is thrown, at which point that code stops and what's in the corresponding catch block is executed instead. It doesn't matter where the exception is thrown (so long as it happens during execution of that try block, of course) so your code, or a function, or a function 10 calls down the stack could throw one.
PHP 5.5 adds the long overdue "finally" block, which always executes at the end regardless of whether the try block completed or a catch block executed.

PHP Code:

try {
echo "I executed\n";

throw new Exception();

echo "I will not execute\n";
} catch (ASpecificException $ase) {
echo "I will not execute because it is a vanilla Exception\n";
} catch (Exception $e) {
echo "I will execute because the exception thrown was an instance of Exception\n";
// (as all exceptions are)
} finally {
echo "I will always execute\n";
}

// I executed
// I will execute because the exception thrown was an instance of Exception
// I will always execute

Originally Posted by English Breakfast Tea

To apply this to the log_in code, perhaps something like this:

Perhaps. Three things:

1. Exceptions are most powerful when you use different types of them for different conditions. If you always use Exception then calling code has no possible recourse for dealing with what are, really, varying degrees of errors. In your example there are three: (1) invalid function call because there are missing values and calling code should know better than to do that, (2) invalid argument because the $email was supposed to be an email address but wasn't, and (3) the function was called with valid inputs but was unable to perform the expected action because the credentials are wrong. Each of those should get their own subclass of Exception so that my hypothetical code could (1) display a general-purpose "error" message and do something to notify me that the code is wrong, (2) tell the user that the email was not a valid email, or (3) tell the user their credentials were wrong.

2. Given that the first three branches of your initial if fit into the #1 "the developer did something wrong by allowing information so obviously incorrect to pass through to my function", just collapse those down to one check.

3. Style choice: since your log_in function doesn't technically require that the $email be an email address (it's not like it tries to send an email or locate a "@" or something), you could do away with that check entirely and know that the SQL will not find a match. If the calling code wants to verify the input is an email address then it can.

When I get the alert about the email/password being empty, I'd see that in my code I didn't bother checking if the email or password were given and then change it so it does.

There's a sliding scale of what you can check and when, but I personally subscribe to the "check when/where it's needed" mentality, such as
- When you want to vary your actions based on the input (like giving a special message for missing input and another for invalid email). This would happen in the calling code because that's where the special message would come into play
- When invalid input can cause problems or unexpected behavior (like needing to send an email to an address or doing a DNS lookup on the address's hostname)
Applying that to your specific situation, I'd only throw an exception when the credentials are wrong - an invalid input is not only the user's fault but totally harmless to your code - and let the calling code check for empty inputs. (Valid email address being completely ignored.)

But it's your code so you pick what feels right to you. I actually worked with someone who was absolutely paranoid about every possible mistake or error and while their code was very reliable it was such a hassle to read through.

The try-catch statement actually comes from object-oriented languages like Java. It's completely different from how PHP usually handles errors, and it's not really integrated into PHP yet. That's probably the reason why so many people struggle with it.

The main purpose of try-catch is to handle errors in a smart way.

In classical PHP, you check the return value of a function to see if it failed:

That's OK for procedural code. If your whole script is just one big file with a bunch of statements, then this is how you do it.

However, modern web applications tend to be a bit more complex than the amateur home pages from the 90s. You may have a lot of classes, deeply nested method calls and multiple layers of abstraction. In this scenario, the old way of handling errors no longer works. What if you want to handle an error at a higher level instead of reacting to it right away? What if you want to handle a whole group of errors? You simply can't do that with return values.

Exceptions are smarter: They represent the error as an object of a specific error class like PDOException or DatabaseException or simply Exception (which is the most generic class). The exception "bubbles up", which means it gets passed on from method to method. Any method is allowed to catch the exceptions of a certain class and handle them. If you don't catch an exception at all, it "bubbles up" to the top level and finally generates a fatal error.

You do not have to catch an exception. If fact, you should not unless you actually wanna do something with it. A common misconception is that every piece of code needs to be wrapped in a try-catch block to do something like this:

PHP Code:

<?php

try {
// whatever
} catch (Exception $e) {
die($e->getMessage());
}

No! This is complete nonsense. And it's a shame that the PHP manual contains a lot of examples doing this.

Remember when I talked about smart error handling? The code above isn't smart. It simply stops the whole script right where the error happens and dumps the raw error message on the screen. The application has no chance of handling the exception at a higher level and possibly recovering from it.

Even worse: The error message will always be displayed on the screen, regardless of whether the error happens on your local test server or on your production site with 100,000 visitors per second.

This is an anti-pattern. It's one of those things people blindly copy and paste without ever thinking about it. As a simple rule: Do not catch an exception unless you actually wanna handle it. If you don't know what do to with the exception, simply let it bubble up. If it's not caught at all, it will automatically send the error message to the right location (the screen, a log file, whatever).

Regarding your (or rather requinix') code, I'm not really sure if this is a good application of exception. Is a user entering wrong data really an application error? Or isn't that rather something perfectly normal?

Personally, I would simply save the error messages in some array and display them to the user. I wouldn't use exceptions, because I feel they don't really fit and make the code cumbersome. But let's not split hairs here. Use whatever you find most intuitive.

Why canít I use certain words like "drop" as part of my Security Question answers?
There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".

What my use of catching Exception didn't explain was that, at that point, any thrown exceptions were being acted upon. It's as Jacques said: you don't catch everything, just the things you want to deal with, and that code was dealing with it. However I can think of a couple, fairly rare reasons why you might want to catch everything:

PHP Code:

$bar = calculate_bar($foo);
try {
stuff($bar);
} catch (Exception $e) {
// maybe you want to wrap this exception in a new one
// (perhaps you want to provide a more specialized exception with more specific
// information about what was happening when the error occurred)
throw new Exception("Could not execute stuff() using foo={$foo}", 0, $e);
// (third argument being the previous/inner exception)

// or maybe there's some weird case where you may or may not want to handle
// exceptions
if ($handle_this_exception) {
// whatever
} else {
throw $e;
}
// but I can't think of any reason off the top of my head why you might want to
// do that here and not further up the call stack
}

The only place where you would (potentially) catch all exceptions is in some sort of front controller location where all code branches out from. Like those frameworks where you route all requests through a single file? That file could have a try/catch in there.
The reason is that an uncaught exception will completely kill the script. As if you die()d. Naturally that's not a good user experience so you'd catch all exceptions at that chokepoint and then dump out some simple (to avoid even more errors) "internal server error"-type page. It's not great, but it's better than a white screen of death.

If you don't have that single file, or can't otherwise use a try/catch block, there's set_exception_handler.

Originally Posted by Jacques1

Regarding your (or rather requinix') code, I'm not really sure if this is a good application of exception. Is a user entering wrong data really an application error? Or isn't that rather something perfectly normal?

Yeah, I know. Depends how you look at it: either you violated a precondition by passing bad data (which is, IMO, a valid but extreme way of looking at it) or you need to run the input through some error checking before you try to act on it. Exceptions work a lot better with the former than the latter, but I didn't want to come up with a different example to explain exceptions. Plus I kinda had suggested to him using them for this purpose earlier so...

My reasoning for doing this in the class would be that I dont have to try to remember/do try/catch every time a script calls this class? instead i can just rely on the $error variable of class to see what happened?

instead i can just rely on the $error variable of class to see what happened?

You're bypassing the whole exception model and replacing it with your own error mechanism which is basically just a fancy variant of the "good" old return values.

That's not good. First of all, it's a nonstandard approach, and it's not intuitive. This means everybody who works with your code will stumble upon it at first. Secondly, it's a "dumb" model compared to standard exceptions, because you cannot delegate or group errors. It works just like the return values. And third, it's a lot of unnecessary work, because you have to write the same code for each and every method in order to replace the native exception mechanism.

The question is: What are you trying to do? Do you just wanna log all fatal errors? Then you don't need try-catch at all. Since you want the same action for every error, register a shutdown function via auto_prepend_file.

Note that it's not enough to simply register an error handler or an exception handler, because those will only intercept some of the runtime errors. If you wanna catch all errors (including syntax problems), you need to have PHP prepend a shutdown handler.

Why canít I use certain words like "drop" as part of my Security Question answers?
There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".

wouldnt it be more unnecessary if i had to do try and catch in EVERY script that calls this class?

If you actually used this approach (which you shouldn't), then you'd only wrap every script which is directly callable by the users in a big try-catch block. Because that's where (runtime) errors eventually end up.

How many of those scripts do you have? Maybe 10? 50? 100? That's not all too much work.

But like I said: A central shutdown handler makes this unnecessary. It will handle each and every error so that you don't have to repeat the same actions again and again.

Not sure what you mean. Where does this false come from? Does it come from some native PHP function to signal an error? And now you wanna turn this into an exception? That wouldn't make a lot of sense, because PHP usually generates an error already if there's something wrong. I'm not aware of a function which only returns false and stays completely silent (but I could be wrong here).

Either way, yeah, you might do this occasionally. But trying to do this for every single return value would be insane. We have to deal with the fact that PHP is a mixture of different concepts from different eras. You have a bit of procedural programming and a bit of OOP. Sometimes you get an error, sometimes you get false, sometimes you get an exception. If you don't like it and want a coherent concept, you'll have to choose a different language.

Why canít I use certain words like "drop" as part of my Security Question answers?
There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".

Either way you shouldn't have much need to reference $_GET/$_POST/$_REQUEST throughout the script. This also has the benefit of allowing you to manually set the local variables for testing without making changes to the superglobals.

1. Exceptions are most powerful when you use different types of them for different conditions. If you always use Exception then calling code has no possible recourse for dealing with what are, really, varying degrees of errors.

Why is that? I see you explained it, but please explain again. How dos that exactly work? This may sound pretty dumb thing to say but your code is longer than mind and I don't really see why it's neccessary to have different Exceptions in this case (or other cases). What is the problem with having only 1 Exception ?

You know how functions can return different values depending on how well it worked? True or false, object or null, number for an error code...

The way to do that with Exceptions is different types of exceptions. Like InvalidArgumentException or UnexpectedValueException. If you throw Exception all the time then it's like returning false all the time: okay, great, you know there's a problem, but you don't know what it was. Was there a problem with the input? Should I tell the user they did something wrong? Is there some serious error that needs immediate attention? No way to know.