kilabit.info
| Ask Me Anything | Build | GitHub | Mastodon | SourceHut

This journal collect the bad coding practices that I have found during my journey maintaining legacy software.

Grouping same class file into one directory

This is a pattern that I always see where developers group two or more files that are not related to each other but have the same "class" into one directory. For example, grouping all controllers into one directory called "controllers", grouping all models into one directory called "models", and so on.

├── controllers
│   ├── feature-A.ctrl
│   ├── feature-B.ctrl
│   ├── feature-C.ctrl
│   ├── feature-D.ctrl
...
├── models
│   ├── feature-A.model
│   ├── feature-B.model
│   ├── feature-C.model
│   ├── feature-D.model
...
├── views
│   ├── feature-A.view
│   ├── feature-B.view
│   ├── feature-C.view
│   ├── feature-D.view
...

This pattern making hard to navigate the source code. When you open the view code, you need to jump to other directory to view what the view trigger, and then jump again to another directory to lookup what the model of data that controller manages.

The good practices is by coupling them by feature,

├── feature-A
│   ├── feature-A.ctrl
│   ├── feature-A.model
│   ├── feature-A.view
...
├── feature-B
│   ├── feature-B.ctrl
│   ├── feature-B.model
│   ├── feature-B.view
    │
    ├── feature-C
    │   ├── feature-C.ctrl
    │   ├── feature-C.model
    │   ├── feature-C.view

...

In this way, the scope that directory provides is limited by feature. We can also make dependencies between features also clear. For example, we can say that feature-C exist only when feature-B is enabled or depends on feature-B to be functional.

Deriving mode from URL path

Given the following URL for editing a record: "/book/:id" and URL for creating a record "/book/create", a single page is created using the same view and controller. The controller check that if "id" exist then the current context of the page is in update mode and the view has an "Update" button. If the ID did not exist then the context of the page is in create mode, and the view has a "Submit" button.

The bad practices is when mixing two different functionalities forced into one component (one controller and one view). The controller and view littered with if-updateMode-else or if-createMode-else conditions, which makes the code hard to read and changes.

The good way to solve this kind of problem is by creating two separate pages with shared form component and two different controllers. The mode and functionality then passed to view component as parameters. For example, on the page that create new book, the form can be instantiated by,

<my-form mode=create on-submit=doCreate>

While on page that update the book, the form is instantiated with

<my-form mode=update on-submit=doUpdate>

In the form, we can still have if-updateMode to disable or hide some fields or information, but at least the condition is clear and does not have mixed functionalities.