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 in return 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 in loops in JavaScript!

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 type undefined (see relevant

  • By using != we compare object (by definition valueless) with string ("") of value undefined?! 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

Did you find this article valuable?

Support Wojtek X by becoming a sponsor. Any amount is appreciated!