The problem with PHP built-in functions is that they are designed with procedural approach. In OO world, you throw exception, but returning false, 0 or null is exactly what procedural functions do. This is an inherent problem with procedural code, and the best solution is to provide OO API for these old procedural functions. For the file_get_contents() example, you rewrite this way(taken from SplFileObject::fread method documentation page):

It throws RuntimeException if the file does not exist or cannot be opened. You may also extend SplFileObject class to add your own methods to it if you need functionality from php file functions that are missing from this class.

If there is an "OO" oriented alternative to using the procedural approach, it is a good idea to use the objects.
But even with the objects, some methods are returning false instead of throwing exceptions... like DateTime::createFromFormat

The more I think about it, the more I wonder if what we really need is not a PHPStan or PHP-CS-Fixer extension that forbids using some "unsafe" PHP methods and that promotes instead proper OO alternatives.

Meanwhile, a thin wrapper like "safe" has the advantage of the simplicity. No need to learn anything new for the developer, it works like the PHP core functions.

Actually, DateTime::createFromFormat() is not OO at all, it is procedural. Static methods may look like OOP, but in nature they are nothing more than namespaced functions. Now try to write a function createFromFormat() in the namespace DateTime, you'd call it with DateTime\createFromFormat(), and you notice this stunning similarity as using static methods.

This approach puts a procedural programmer back to his comfort zone, he is effectively writing procedural code masquerading as objects. It does not surprise me at all that the static method DateTime::createFromFormat() is not following standard OO practices, since it is not really OOP at all. In my application I dont use static methods, when I feel like using them I realize my design is flawed and I need to rethink what I should be doing.

In general you are correct. Avoid using static methods when anything that manages state is involved.

However ::createFromFormat() is nothing more than a factory method. It does not involve or manage state, but simply builds and returns a DateTime object.

It‘s fine for factory methods to be static and return an instance of whatever the factory is meant to build. If you have a factory method that returns an instance of itself, it would make absolutely no sense to not make it static.

Even if they do not modify global/static states, these 'pure' static methods still have one problem that they are placed in classes that they do not rightfully belong to. As a consequence, the class is now responsible for what it is designed for, plus a new responsibility of managing its creation methods. This violation of SRP is no different from how Singleton classes violate SRP, though not as bad as Singleton it is still not a good practice.

So where do factory methods belong? In PHP, you write factory classes and instantiate factory objects to manage creation of objects when the logic gets complex, as many design pattern books have demonstrated that. In a purer OO language like Python/Ruby, classes themselves are objects, so the classes are the factories for their corresponding objects. Its way more elegant as the factory methods fall into the metaclass of factories, as you can see from the examples below:

As you can see, the responsibility of managing creation of Date object is now on the factory class(PHP/Java/C#) or metaclass(Python/Ruby), which is how it is supposed to be done in OO. If you use static methods however, you are effectively doing procedural programming.

From a purist's point of view, you shouldnt be using static factory methods at all. But realistically, its fine if your class has one and only one such method. Once your class grow and you end up with more than one static factory methods in a class, refactor it into a dedicated factory class.

I agree. I don't use static methods for factories either (I don't see the point) but it doesn't shock me if I see it. Of course if the factory is totally bloated with static methods, this is another story.

Admirable effort but some functions such as strpos() should behave as they already do IMO. Like if I want to check if a substring is contained by a string and it doesn't, I wouldn't want an exception thrown thank you very much.

In order to make a difference between functions that return false "on failure" and functions that
return a boolean (like in_array, array_key_exists...), I parsed the PHP documentation for the term "return FALSE on failure".

So it should behave correctly. For instance, strpos is out of scope of "safe". However, the lib is still in the very early stages of development, so there might be some false positive that slipped under the radar.

Absolutely! Converting errors into exceptions is definitely the right thing to do. I'm actually talking about it in the blog article at the "Using strict error handling" chapter.

And I most definitely think you should always use strict error handling, with all errors converted to exceptions and error_reporting=E_ALL.

But I see 2 cases where "safe" is bringing an improvement:

There are situations where you are not in control of error handling. If you are writing a PHP library, you don't know what error handler your user is using. Also, most error handlers will be sensitive to the "error_reporting" setting. Hence, depending on the environment, your program may behave in different ways. On the other end, an exception is always thrown the same way. This brings predictability.

"safe" is really useful for advanced static analysis tools since the functions you are calling cannot return false in the eyes of the tool.

Sorry but I strongly disagree. The file_get_contents() function is only one case among others. If you get a look to the library (maybe you haven't), you'll see that most of the functions returnFALSEwithout triggering any error in which case you cannot handle the issue globally.

There fore this lib allows you to detect possible errors, and have a clean error reporting. Take base64_decode() for instance, I'd better see that I must handle a potential Exception than getting unclear message triggered afterwards like 'expecting string a but a boolean was passed'.

​

Thank you u/moufmouf for your work. TMO this package is pushing in the right direction for cleaner code.

This article begins with the assumption that throwing exceptions is better than returning false, but it doesn't explain why this should be so. Most of the programming I've done is procedural, and I'm not too familiar with how exceptions work or what they can do for you. But if a function does return false on failure, I know how to handle that, and it doesn't seem like something in need of fixing. For example, I have used file_put_contents() in a conditional statement that includes an else in case of failure. This is simple and straightforward. How would using exceptions improve on this?

I have used file_put_contents() in a conditional statement that includes an else in case of failure. This is simple and straightforward. How would using exceptions improve on this

This is simple, and if you think about putting all the functions that can return false in a conditional statement, this does the job.

But it puts the burden on the developer to think about doing the error handling correctly. And it is very easy to forget doing the error handling. Also, it is kind of hard to do error handling in a state-of-the-art way anywhere in your code.

For instance:

php
if (!mkdir('somedir))) {
// Maybe I should return a proper 500 HTML page, instead of dying here.
// But if this code is in a service layer, is it ok to print HTML here?
die;
}

The exception will halt the execution of the current function. It will jump to the "catch" statement if there is a catch statement. If there is no catch statement, it will return to the calling function. Is there a catch statement? Go to catch statement. No catch statement? Go to calling function... and so on.

Assuming there is no try catch statement in your application, the exception is identical to a "die" (with the added benefit that you get a "stacktrace" telling you where the exception occurred).

And if you want to explicitly handle the exception (maybe because you have a plan B in case an error happens), then you can "catch" the exception.

So to put it in simple terms, when you application throws an exception, it will "die" unless you specifically decide to handle the error. Compare this to returning false. If a function returns false, your application will continue unless you tell it to die.

I really prefer having a process that dies by default when an error occurs, than an application that hides the dust under the carpet.

Most OO languages have exception support and it is very similar in PHP, Java, C# or C++.
Other languages like Go or ElmJS have different approaches (like forcing you to handle the returned values of the functions) but these are not the approaches chosen for PHP.

Assuming there is no try catch statement in your application, the exception is identical to a "die" (with the added benefit that you get a "stacktrace" telling you where the exception occurred).

I understand the benefit of tracing where a function was called. With that in mind, I looked up how I could do this in PHP, and I included the line error_log (json_encode(debug_backtrace())); in a function. It then told me where the function had been called from, which led me to debug the script that was calling the function with unexpected data.

So to put it in simple terms, when you application throws an exception, it will "die" unless you specifically decide to handle the error. Compare this to returning false. If a function returns false, your application will continue unless you tell it to die.

So, if all core functions in PHP were modified to throw exceptions, there would be the danger of scripts suddenly stopping when they would have otherwise continued. While this might be desirable for some scripts, it could also cripple scripts that were otherwise functioning well-enough. Also, failure is sometimes to be expected, and I wouldn't want a function like strpos to end the script because the search string couldn't be found in the searched string. This leaves me wary of making the core PHP functions all act this way.

This article begins with the assumption that throwing exceptions is better than returning false, but it doesn't explain why this should be so.

It might not be a very good reason, but for me exceptions are better because there are a lot of cases where false is actually a valid result and not an error. So there's no easy way to differentiate between the two.

Consider handling this undocumented results (probably there are more array functions behaving like that). I'm not sure whether it should throw exception or maybe silently convert input null to empty array.

Fair enough, if the scope of "safe" is set of functions which return FALSE on errors, and not a bigger set of functions which return something specific on errors (in case of those mentioned array functions, that "something" is NULL, not FALSE).

It's only now that I saw this library and I am blown away by the effort.
This is how native functions should behave.
IMO you should push this to the PHP core so maybe we can see this on PHP 9 or 10.
At least there should be an ini setting to switch this on on future PHP versions.

Thanks for the effort, this is easily the next best thing in PHP since it became OOP'sh.

Reading past the end of the file is not exceptional - it's normal, boring, mundane.

I honestly don't know what this means. How do you read past the end of a file using file_get_contents? I've never ran into this failure mode, and I can't find any reference into the article.

One common case where file_get_contents returns fail instead of throwing an exception it's when the file doesn't exist. And if trying to open a file that doesn't exist it's not exceptional to you... then I don't know what it is.

Changing the interface of a standard PHP function is just bloody stupid, to be honest - you're essentially breaking the PHP docs

I'm not defending the library, I agree with you here. But that's not what your original comment was about.

Whether a result is exceptional depends on what expectations the consumer had beforehand. It's rather Bayesian that way.

If my applicationcs config file is missing, that's an exception, the only question is how deeply down the stack that should occur.

P.S.

Speaking more towards the linked post:

A particular special-return-value situation commonly causes bugs.

This indicates actual users of that API didn't expect the edge case, it wasn't on their mental radar.

Therefore the situation is exceptional.

There's a certain aspect of "the customer is always right" here. A huge chunk of "good" API design is correctly guiding and providing for the actual user base who show up, and changing that API when you discover those users don't think like the original developer or use the tool in the ways the developer imagined. (Usually those changes are to accommodate their use-cases, but sometimes it can also be to direct them away to other tools.)

If anyone wants a non-sarcastic take on the debate, google "Exceptions are Exceptional" and see what people who wrote blogs ten years ago thought about this

I've usually heard that in the context of C++, since exceptions can actually cause a performance impact that, in some cases, is non-trivial. If the application needs to run as quickly as possible, exceptions should indeed be used exclusively for scenarios where you want the program to crash, burn, and tell you what happened.

PHP? It's an interpreted language, there are lots of performance penalties you're already dealing with. Exceptions don't add anything significant in this context, and if they can make the code more readable and easier to follow/use/reuse, why not use them?

They take the handling of the error far (sometimes very far) away from where the error was raised, and that's bad for your readability metrics.

Well, they supposedly contain a stack trace showing which function it occurred in, which function called that function, which function called that function, and so on. By this logic, error codes being passed through multiple return statements before hitting an if block looking for it are far worse.

What defines that line? For me it's simple: if we can recover from it locally, we shouldn't throw an Exception for it.

Define 'locally'. PHP is server-side, so to the code the server is 'local'. Do you mean 'within the same scope'? What if something requires more processing of the inputs before determining if it's recoverable or not?

Perhaps not receiving an email address is fine, as long as a phone number is provided instead - and phone number processing is performed in a different scope (function checkContactInfo() calls checkEmail() and then checkPhone(), for instance). Since the actual checking is no longer possible in the current scope (within checkEmail()), does that mean an exception should be thrown? I don't think so.

Do you mean 'within the codebase being worked in and not in the libraries/frameworks used'? Does that mean you should wrap everything that calls a function written by someone else in a try/catch block? I don't think that's the case either, and completely sidesteps the question of whether you should throw an exception or not in a given scenario.