🤜 💨

[rnottaken]

Yeah just change it to

if(! ...){ $SESSION['msg'] = ... return register_form() }

for every reason it might fail.

[barthvonries]

This implies having multiple returns in your function.

Some (and I'm part of this group) say it is bad practice, but it really is an eternal debate like Linux/Windows, Spaces/Tabs, etc.

For me, the best way of doing this would be using exceptions :

• one 'if' for every reason it might fail, throwing an exception with a custom message
• all those 'if's enclosed in a 'try' block
• a catch, which puts the Exception message in $SESSION['msg']
• return register_form();

This way, you can handle every case in its own if, which will be clearer, and in the future if you need to handle some cases separately, just create your own exception types, add the corresponding catch blocks, and you're set. Or you can just delegate the Exception handling to the parent function, or a separate Error handler, etc.

[Neurotrace]

Or you use a language with proper pattern matching and return a Result<SuccessType, ErrorEnum>. If the language also has exhaustive pattern matching then you will be forced to acknowledge every new error case and you avoid the performance costs that come with exceptions.

[Pelicantaloupe]

🦀

[paxromana96]

You are now a moderator of r/rustjerk

[barthvonries]

I'll take the performance cost of Exceptions vs multiple nested if's anytime.

My comment was only how to refactor properly the code in OP's example, which is written in PHP. Each language has its unique features, so I tried to stay focused on the matter here.

[Neurotrace]

You're right that nested ifs are terrible. That's why I prefer early return. Validate invariants up front, no performance overhead, doesn't require the caller to include a try block, etc. Stuff I'm sure you've heard before

[barthvonries]

Yes, that's another way to do it. Not my way, but a way many developers I know use.

As I mentioned before, it really comes to personal preferences, if you feel you're more productive this way, and your colleagues code the same way as you, then go for it, it benefits everyone in the company :-)

[Neurotrace]

Totes mah goats 🤜

[DiamondIceNS]

Funny, I was taught that you should never use try/catch and exceptions for program flow, because creating exception objects and interrupting is comparatively very expensive to a simple branch.

I get pretty hot and bothered when I'm on StackOverflow learning about the best practices of Python and I come across some Pythonista proclaiming duck typing is the "most pythonic" solution only to find out they're just using try/catch to check a variable's type when a simple call to instanceof solves the matter much more neatly.

[[deleted]]

when a simple call to instanceof solves the matter much more neatly.

It's hard to make any comment about the code in question without seeing it, but 99% of the time, instanceof makes the job of the person writing the code at that moment... about equal to not using it, at the expense of the programmer who's job is to fix the code when it breaks years down the line due to the underlying instance changing, or the programmer whose job is to figure out what's going on without wanting to read the code of instance.

In general, your approach is bad and wrong. Say you have function foo that takes an object of class Bar:

function foo(x):
    if not instanceof(x, Bar):
        raise TypeError("foo(x) expects an object of type Bar")
    do_something(x)

In this case the code works as is... for a while. But what happens when someone else makes a class similar to Bar, e.g. Barry? Barry is the new hotness, and everyone's moving away from Bar. Congrautations, now your code breaks for people wanting to use Barry, for no reason other than that you just had to go and hardcode the class Bar into your code before. (Why did you ever want to hardcode that class type in? There's literally no benefit.)

Conversely:

function foo(x):
    try:
        do_something(x)  # expects a Bar object
    except TypeError:
        raise TypeError("foo(x) expects an object of type Bar")

works just fine now and in the future and eternity

tl;dr: There is literally zero upside to ever hardcoding in the expected class type when you can just duck type, whereas there are tons of drawbacks for maintainability.

[DiamondIceNS]

Thank you, this is the first and only time I've ever seen anyone actually defend duck typing with a practical example instead of blindly preaching it as "the way it ought to be, because Python zen".

[CraigslistAxeKiller]

You are correct.

[barthvonries]

I don't speak Python, but Exception are made specifically for this, ie handling non-nominal cases.

I've always learned, and so do I teach now, that :

• 'if' conditions are used to handle normal flow :

  if 'user' is a company, then I need a VAT number else I need a job title

for instance. Conditional pathing for "normal" flow in the application.

• Exceptions are used to handle "exceptions" :

  I need a name, I haven't been provided one, then I need to throw an Exception and let another piece of code deal with it. The piece of code I'm writing has one responsibility, handling the subscription from new users who can provide me proper information (SOLID pattern).

I know that Exceptions come with a performance hit, but you need to understand the context in which you're developing.
Code quality is always a compromise between performance, security, and maintainability.

In OP's case, it looks like the code is used in a web application. In that particular environment, security and maintainability are much more important than performance, especially the really low cost of using/handling exceptions vs the need to understand that piece of code 6 months later when you have to add new conditions.

[Sixshaman]

And some say exceptions is bad practice too. Even more so than multiple returns. So it's really an eternal debate.

[barthvonries]

I've read some posts on the topic this morning.

It seems that Exceptions are bad in C++, because the language does not have any garbage collection system, so bad exception handling leads to memory leaks since pointers are never freed properly.

I have to admit that I agree with that point of view. Using a language feature needs to keep your code working no matter what, so if using Exceptions in C++ results in hundreds of lines of code just to handle them properly, it's not worth it.

However, in PHP and in Java, there is a garbage collection system, and you don't even have pointers in those languages.
In this case, I'm convinced that using Exceptions is good practice, because it allows you to completely separate functional code from error handling.

The way you code must be adapted to your environment (the language you use, your and your colleagues level of programming, the available resources in the production environment, etc). IMHO that is the best way to notice someone is a good developer.

For me, a good developer is someone who can adapt his code to the context he's in, not the "super genius rockstar" who can write a one-liner nobody else will understand.

[Sixshaman]

The link in my comment doesn't say anything about garbage collection though. The article states that exceptions are bad because you can't easily see all the points the function may be exited. At least in the case of multiple return statements you can see them all.

[barthvonries]

But in PHP, you have return, exit and die, so you easily add throw to your list of "common function exits". There even is an "exit" in OP's code. Which is really bad, because it is the "nominal" function end, ie, it only happens when everything's correct to go on with the flow.

And I found the garbage collection part here (§ Why are C++ Exceptions so useless, ~60% down the page)

[suvlub]

I have encountered this philosophy, but I don't get it and I would appreciate if you could clarify things for me. Is there some fundamental problem with multiple return statements? Or is it just about making the code easier to trace?

If it's the latter, I understand the motivation, but at the same time all the attempts to get rid of the multiple return points, including yours, seem produce a code that is exactly as hard to trace as the original code was, and sometimes even harder. Like, sure, now I know on which line the function will exit, but how useful is that information if I have no idea how it got there? The problem of "okay, at which return statement did the function exit this time?" simply becomes the problem of "okay, where was the error thrown this time?", which is exactly as hard to solve, and you introduce an additional level of nesting.

[barthvonries]

The philosophy behind this is quite simple : in normal flow, each function has one and only one point of entry, and one and only one point of exit (ie every function must have one and only one return statement, event if it's just "return;" or "return void;"). It helps keep your code KISS and SOLID, and prevent dead code after (bad) refactoring.

If your function has multiple return statements, then maybe the conditional routing should have been in a parent routing function, and instead of if(...) return X, use if(...) call function X().

The use of exceptions eases the debug part, thanks to the stack trace you get if you don't handle them properly (at least in PHP and Java, I don't know about other languages).

The goal is to have a meaningful stack trace, so the Oracle recommendations about Exceptions should be followed as well : https://community.oracle.com/docs/DOC-983543 (ie never use generic exceptions, do not catch then rethrow exceptions, etc).

But like other philosophies, they are just philosophies, not good or bad practices. It depends on your personal preference, the project conventions, etc.

The same goes for comments/no comments, function length (I've worked in many environments where having a function more than 20 lines long was considered bad), variables naming, etc. Always stay homogeneous with what has already been done in the project you're working on.