Clean Code concepts adapted for PHP
github.com
github.com
How to do for and foreach loops for example: https://github.com/jupeter/clean-code-php/commit/08d73d655ef...
For all the good that Uncle Bob and the like do they have become enablers for these snake oil architects.
Bad:
$ymdstr = $moment->format('y-m-d');
Good:
$currentDate = $moment->format('y-m-d');
Both are inferior to "$ymd". Use searchable names
addExpireAt(DateGlobal::SECONDS_IN_A_DAY);
Who would ever need to search for that? You'd search for "Expire". This doesn't preclude from using named constants, but it has nothing to do with search-ability per se. Don't use flags as function parameters
Good:
function createFile($name) { touch($name); }
function createTempFile($name) { touch('./temp/'.$name); }
Now, this is how you create the good old copypasta.What you need instead is a single universal function that does in fact accept a boatload of parameters, but that is never meant to be called directly. You then supplement it with a host of simpler one-liners that fill in most of the parameters with defaults and forward just the select few.
Avoid negative conditionals
should be "Avoid double-negative conditionals". Avoid conditionals
this is just overly broad to the point of being completely pointless. What the example proposes is to use abstraction and inheritance instead. In some cases it helps, but it many it leads to needlessly obscure code that's impossible to read off the page. Use method chaining
Good:
$car = (new Car())
->setColor('pink')
->setMake('Ford')
->setModel('F-150')
->dump();
Except at some point you will need setModel() to fail for some arguments, e.g. setModel('Trabant'), and you will end up with some methods returning $this and some not. This is not going to be a "clean code" by any measure.(edited for clarity a bit)
I half-prefer `$currentDate` but then it doesn't really contain the essential "it's a YMD string" information. `$currentYMD` would be better than all of the above, IMO.
All of which assumes you religiously stick to "`...YMD` means a 'Y-M-D' formatted string" and "`...Date` means a date object", mind, otherwise you might as well pick random strings.
That information is not essential. If the date is later changed to also take hour/minute etc, it will still be currentDate but not YMD.
Unless you need to work with the structure of the date, and not just pass it along, currentDate is better.
It is if you're passing it to something which expects to receive a YMD date.
Also a `$currentDate` which also contains the time is borderline acceptable if specifically called out in comments / docs but I would still ding it in a review.
You don't want to expose what kind of date string that something depends upon in your variable name.
This way you you can change the internal implementation (what you pass and the function that received it) and not 1000 places where it's called.
Sure you do if you're a subscriber to Hungarian Notation. And if you're working in a loosely-typed language, it helps a lot to be able to track types through variable names.
Hungarian notation encodes types (and/or scope etc) -- not formatting details. Where would it stop if the latter was the case?
>And if you're working in a loosely-typed language, it helps a lot to be able to track types through variable names.
ymd is not a type though. str_currentDate is.
That entirely depends on your interpretation of Hungarian - there are many. Plus "YMD" is easily valid as a type - imagine creating a struct with { y int; m int; d int; } and using that instead.
> ymd is not a type though.
Only because you have decided it isn't in this instance. There is no obstacle to it being a type.
I think this is wrong, in my opinion the correct is $formattedDate = $moment->format(translation('date-format')); since how a date is formatted will depend on the current language, you don't need to format it if it is for internal use
Or if you have a language that can have supply defaults for parameters set them to the default case?
foo(1,,,'abc',,)
Meaning that if you need to override a default for, say, 2nd argument, you will also need to explicitly provide all arguments that follow. function bar($word1, $word2='loves', $word3='Elixir') { return sprintf('%s %s %s', $word1, $word2, $word3); }
$foo = bar('Hacker News', default, 'PHP');
But I guess functions with too many arguments are frowned upon and it would encourage it.Better yet in php 7 using the splat operator ...
foo(...$args)
Allows you to send in any number of args.
Thankfully PHP now supports exceptions, which all chain method APIs have used exclusively used. It would be hell if they a mixed value consisting of true in case everything is ok or any other type (including strings, ints and custom objects) depending on the return code. If you used such an API, you would have spaghetti code because every other line would be if ($err !== true) { handleErr }
It is just much easier to have a try and catch block and use multiple catch and try blocks if you need more fine grand controller where the error occoried and the exception does not contain that detail.
So in a way, chained method should always garantuee to return itself or throw an appropriate error.
Reminds me of https://cirillocompany.de/pages/anti-if-campaign --- not sure if satire.
Nope. $ymd encodes information about the formatting of the date that could just be an implementation detail (and might change, e.g. need to have the hour/minute too later, or have the month first, or be encoded in a different timezone etc). Whereas that it's the "current date" is a business predicate, and more relevant.
>addExpireAt(DateGlobal::SECONDS_IN_A_DAY); Who would ever need to search for that? You'd search for "Expire"
I might search for all kinds of things. A searchable name is better. It's also a good way to write the constant, whether you search or not.
>Now, this is how you create the good old copypasta.
Depends on how many options there are and how crucial is what they do. Two clearly defined functions can be better than an "abstract" function and two instances.
>should be "Avoid double-negative conditionals"
No, it's negative conditionals alright. The example to be avoided is: "isDOMNodeNotPresent" -- where's the double-negative?
>Except at some point you will need setModel() to fail for some arguments, e.g. setModel('Trabant'), and you will end up with some methods returning $this and some not. This is not going to be a "clean code" by any measure.
You can trivially use an Exception to signal that. And in other languages, an Optional/Maybe etc type.
Yep.
The original "bad" case was meant to capture the fact that the formatting is YMD. The "good" example however is stuffing a string with an YMD formatted date in a generically-named variable.
> A searchable name is better.
Nobody argues with that. Point being that search'ability of named constants is not the principal reason for using them.
> Two clearly defined functions can be better.
Unless they substantially overlap in their code, in which case it's not better.
> No, it's negative conditionals alright.
"isDOMNodeNotPresent" is not a _conditional_.
> You can trivially use an Exception
Certainly. But for every dev who thinks that exception-based error handling is a good idea there will be two who don't. And that's even without getting into whether it will be a clean code.
Depends. DRY is not an absolute rule. Sometimes the added abstraction to make something DRY makes it messier.
>"isDOMNodeNotPresent" is not a _conditional_.
Well, the if is assumed, no? What the author suggests is to avoid having what you check being negative -- so not only "if (!$isNotFoo)" but also "if ($isNotFoo)" is bad as well "if (!isFoo)" is clearer imo.
>But for every dev who thinks that exception-based error handling is a good idea there will be two who don't.
Some people don't like checked exceptions, but I don't think many have beef with exception based error handling.
I initially thought this was going to be the use something else meme - http://devhumor.com/content/uploads/images/December2016/php-...
Querying for data will be the bottle neck. Until you move to some in memory solution trying to reduce the number of classes called to increase speed will have little effect because that will be cached anyways.
That's not how modern PHP works.
That article isn't great, but okay enough for this discussion.
There's a great deal of interpreter level caching + object caching (in more advanced system) that allows modern PHP to reap a lot of the benefits of the "everything fresh every request" model while also being faster than pretty much anything but Lua and Perl.
It's like HackerNews BINGO.
Savage, but not entirely unfounded. Uncle Bob and his promoters try to keep an aura of Zen master around him, but there are no deeper insights in his truisms. "Clean Code" is nice for a novice coder, but dangerous if taken to an architectural or inter-module level.
Having more clearly named everythings and more limitations in the API's I write for myself really help me maintaining all of the things that I've been writing in the last decade or so. If I'm in my IDE I expect that when I have an instance of an object and I type `.` (or it's equivalent) it will autocomplete with only the things that I should touch and those things should make sense immediately without having deeper knowledge of the object I'm consuming.
Languages like PHP but Java and C# a bit less but just as well are prone to writing code that can confuse the consumer. More modern or functionally designed and stronger typed languages can take guessing and confusion away, combined with the ideas expressed in Clean Code.
But a lot of other sources as well, don't treat the guy like he's god. I love to read the analyses of Carmack's code for example.
I just keep seeing it being labeled bad a lot. I guess it depends if you can avoid it right? The current company that I work for (contract hourly) is a LEMP stack built on WordPress. Ideally I could strip WordPress out and get to the core purpose of the web application but it's not at that stage yet. I like the idea of separated services but
Edit: I saw some infographic saying PHP was too easy to use/get into and resulted in many bad coders who were using it. It's also old, whenever I'm on the manual site and see the comments dating back to 10 years haha. What about machine code running on computers.
No, of course not. There is no perfect language that you should be using right now. Like anything in software engineering, it's all relative to what context you are using it in and who is wielding the tools in the final end. Buggy, slow and terribly written code can be done in any language.
One thing I find difficult is passing onto another language functions/processes.
One example use would be encryption/dectryption I used a library (defuse/php-encryption) not complaining about and I'm probably guessing here, but hearing about Go's speed, one thinght was what if I passed the data to be encr/decr into Go as a separate process. But how do you get a call back if you needed one. It's an interesting problem. I like that separate processes idea.
Expressing the same idea in PHP vs Ruby or Python always requires more typing.
Let's assume you are going to write really clean PHP code, with excellent readability, thorough tests, appropriate abstractions, and proper encapsulation.
Understand first that it will be unnecessarily difficult: PHP lacks a number of concepts and constructs that allow you to write better code, and does not really reciprocate with sensible trade-offs that make the loss of these features worthwhile. It makes bad code easy to write and hard to fix.
Then consider any third-party modules you might want to import. The standard libraries and modules in Python, Go, C# etc. are likely to be well-written, well-tested, and will fit into your clean-code project nicely. Sure, every language will have bad third-party code available, but PHP is especially likely to have cruddy tooling messing up any
Finally, assuming you work past all the pitfalls (while still avoiding over-engineering), your beautiful masterpiece will still be hampered by the fractal of bad design [1] which is PHP itself. Your comparisons may be literate, the variables clearly descriptive and in the proper positive tone, but you'll be asking if they're empty(), is_null(), and isset().
Why start with a handicap? Choose a better language if you want to write clean code.
[1] https://eev.ee/blog/2012/04/09/php-a-fractal-of-bad-design/
all the pages are blank