AboutCarbon based humanoid lifeform that likes other carbon based lifeforms (most of these seem to be of the non humanoid variety and biassed toward furry or feathered ones). Natures joke: I'm allergic...
SkillsProgrammer proficient in most languages. prefer Go. Also a fan of Ansible and Linux/UNIX. Used to be a systems and network admin.
Joined devRant on 3/1/2017
Do all the things like ++ or -- rants, post your own rants, comment on others' rants and build your customized dev avatarSign Up
Not sure if it's the worst code review but it's a recent one.
We don't really do code reviews where I work unfortunately but my coworker used my framework for the first time (build some nice composer libraries for cmdline projects) and asked if I could make them do autoloading.
He never used namespaces before so I was glad to help him out.
What I saw was a dreadful mess. His project was called "scripts" so good luck picking a namespace...
Than it was all lose functions in the executable file. All those functions are however called by a class in another file (if they where not calling eachother as a cascading mess). That class was extending an abstract class from my library as instructed. However I never imagined my lib being raped like that.
The functions themselves are a horrible mess. Nothing uniform completely different style (our documentation states PSR's should be used).
Parameters counts higher than 5.
Variable names like Object and Dobject (in calling function Dobject is Object but it needs a fresh one.
If statements on parameters that need basically split it in two (should simply be to functions)
If else statement with return of same variable as a single line (sane people use ternary for that)
Note that I said functions. All of it should have been OO and methods. Would have saved at least some of the parameter hell.
I could go on and on. Do I think the programmer is bad yes (does not even grasp interfaces, dep injection, foreach loops). Is this his best work no. He said that for a one of script like this it just has to work. Not going to be used elsewhere. I disagree as it is a few thousand lines of code that others have to read too.2
TL;DR I'm fucking sick and tired of Devs cutting corners on security! Things can't be simply hidden a bit; security needs to be integral to your entire process and solution. Please learn from my story and be one of the good guys!
As I mentioned before my company used plain text passwords in a legacy app (was not allowed to fix it) and that we finally moved away from it. A big win! However not the end of our issues.
Those Idiot still use hardcoded passwords in code. A practice that almost resulted in a leak of the DB admin password when we had to publish a repo for deployment purposes. Luckily I didn't search and there is something like BFG repo cleaner.
I have tried to remedy this by providing a nice library to handle all kinds of config (easy config injection) and a default json file that is always ignored by git. Although this helped a lot they still remain idiots.
The first project in another language and boom hardcoded password. Dev said I'll just remove before going live. First of all I don't believe him. Second of all I asked from history? "No a commit will be good enough..."
Last week we had to fix a leak of copyrighted contend.
How did this happen you ask? Well the secure upload field was not used because they thought that the normal one was good enough. "It's fine as long the URL to the file is not published. Besides now we can also use it to upload files that need to be published here"
This is so fucking stupid on so many levels. NEVER MIX SECURE AND INSECURE CONTENT it is confusing and hard to maintain. Hiding behind a URL that thousands of people have access to is also not going to work. We have the proof now...
Will they learn? Maybe for a short while but I remain sceptic. I hope a few DevrRanters do!7
So yesterday I said to my private laptop update and shutdown...
Fast forward to this morning. Hell breaks loose. Have to fix it asap! We have downtime. But fucking windows update!!!
You fucking peace of shit should have done this yesterday. And why does it have to take so long.11
I have just thought of the perfect solution when support for fucking ancient IE versions creaps in the requirements (and asking the assholes to produce numbers to support the crazy does not help)
Just do browser detection and if IE < 9 Replace body with one of those winXP alert boxes that tell them there Computer is infected and that they can get a free scan (it's what they are used to anyway). Put a link to the installer of your favourite browser over the entire image.😎
Good news is 100℅ code reuse! Works on every outdated IE and every website that requires IE support.4
Every once in a while the flexibility of dynamic types comes back and bites you in the ars:
So I created method that returns the date significance (day, month, year) or null when no date is set.
I chose a class constant DAY with the value 0.
This is not a problem in if statements as I always use === but in this case a switch made more sense. And as you can guess no date set (NULL) was handled in the self::DAY (0) case... 😐😑😶 Silently resulting in wrong results when no date is given! #£#@$& (and other comic swearing symbols)
Even though php7 finally has decent type hinting resulting in much clearer defined API's we can still go very wrong.
More love to Go for less verbose static typing! To bad we can rarely use it at the office 😥2