Bad code
Preface
Thats what Im "allergic" to.
While browsing VCS
like GitHub, 9 out of 10 times I encounter code written horribly. I fully understand that VCS
is built to host OSS code (everyone can edit/add), I also understand idea behind standards (although I dont understand/agree with standards themselfes).
Will not name projects that snippets come from ( just not to harm/shame them ), but will give two things:
example (as snippet)
my commentary with explanation why I think so, and how it should be written
OK, lets start.
Definitions
Standard
Standard is divided as:
Voluntary consensus standards, which are standards developed or adopted by voluntary consensus standards bodies, domestic (national), regional and international.
Industry standards, also referred to as private standards, which are standards developed in the private sector but not in the full consensus process, typically requiring a financial contribution.
Government standards, which are standards developed by the government for its own uses.
Paradigm ( in software development )
Some paradigms are concerned mainly with implications for the execution model of the language, such as allowing side effects, or whether the sequence of operations is defined by the execution model. Other paradigms are concerned mainly with the way that code is organized, such as grouping a code into units along with the state that is modified by the code. Yet others are concerned mainly with the style of syntax and grammar.
Things to note
Extended function()
's return
return ({ dispatch }) => next => (action) => {
if (action.type && !action.skipLoading) {
const [PENDING, FULFILLED, REJECTED] = promiseTypeSuffixes;
const isPending = new RegExp(`${PENDING}$`, 'g');
const isFulfilled = new RegExp(`${FULFILLED}$`, 'g');
const isRejected = new RegExp(`${REJECTED}$`, 'g');
if (action.type.match(isPending)) {
dispatch(showLoading());
} else if (action.type.match(isFulfilled) || action.type.match(isRejected)) {
dispatch(hideLoading());
}
Three problems here:
if
encapsulated inreturn
statement; although structure like this is correct ( will compile OK ), this appoach is highly discouraged by any serious dev. There are two reasons for this:code readability as well as maintainability,
KISS
,DRY
,WET
, and other paradigms ( not the same as standards ) not followed at all.
formatting issue; empty line number 3, breaking
const
block in 1/4. Seriously?condition to check against;
if
statement. Negation has a precedence over reference inloops
inJavaScript
!
Formatting
There are litterally hundreds of projects with fucked up formatting. Most commonly made mistakes are:
breaking code blocks (
loops
,var/const
definitions),wrong/multiplied (many conf files ) configs of
linters
as well as other tools,not following paradigmats ( do not mistake them with standards; paradigmats are extremely usefull thing ); or following multiple contradictory paradigmats in one project.
Example(s) of horrible formatting below:
public function trace(Event $event): void
{
$telemetryInfo = $this->telemetryInfo($event);
$indentation = PHP_EOL . str_repeat(' ', strlen($telemetryInfo));
$lines = explode(PHP_EOL, $event->asString());
file_put_contents(
$this->path,
$telemetryInfo . implode($indentation, $lines) . PHP_EOL,
FILE_APPEND | LOCK_EX
);
}
public static function registerSubscribers(Subscriber ...$subscribers): void
{
foreach ($subscribers as $subscriber) {
self::registerSubscriber($subscriber);
}
}
private const EXPECT_CONTENT = <<<'EOF'
--TEST--
EXPECT test
--FILE--
<?php echo "Hello dear user"; ?>
--EXPECT--
Hello PHPUnit!
EOF;
private string $filename;
private ?PhptTestCase $testCase;
private AbstractPhpProcess|MockObject|null $phpProcess;
private string $filename;
private ?PhptTestCase $testCase;
private AbstractPhpProcess|MockObject|null $phpProcess;
Wait, what? Variable type
starts with question mark (?
)? Second line. Or one var
is able to be of two types
? I know php almost by heart, but have not seen/heard of var being multiple types. What the fucking fuck?
Consistency
This is huge thing. When done wisely.
Indentations
Many devs do not keep indentation within block the same. They indent one line, dont indent the next, and so on. Code written in such way is:
hard to read,
even harder to maintain,
hell to relay
Comments
Same as above. Well written/thought out commenting strategy is circa 50% of future project success. Many forgets this. Comments are useful regardless language we code in.
Overfactoring
Thats whole another thing. Suppose we code in JavaScript
and we want to check if PublicKey
is of correct type. Pretty straightforward, right? NO. Examine:
if (typeof(PublicKeyCredential) != "undefined") {}
Shit! Just awful fucking shit not code! Dont use this.
So many things with this code:
formatting: loop body should be on newline,indented by
TAB
,God knows wether
PublicKeyCredential
is initialized or not. From how its used here, it seems that answer is NO. Even if, somehow, it is, it cannot be of typeundefined
(see relevantBy using
!=
we compareobject
(by definition valueless) withstring
("") of valueundefined
?! Wait, what?
Using multiple tools that serves one purpose
Oh, thats making me extremely furious as a developer and banned from orgs on GitHub
at the same time. When I see project that uses as many tools as possible for the very same one purpose. Like: lets use as many linters/CIs/whatever and see what it all does.
In reality it:
introduces maintainability, dependency, code, security hell,
scalability problems,
says that authors are complete rubbish developers