4
\$\begingroup\$

After some time poorly designing my web applications' backends (mixing database calls with the controller, etc.), I have decided to try the "Clean Architecture" approach.

In this example I have a REST api users which allows you to get a list of all users in MongoDB and to put a user inside the database.

Please, any suggestion on how I can make it better organized would be awesome.

app.js

const express = require("express");
const bodyParser = require("body-parser");
const config = require("./config.js");

const Routes = require("./src/routes.js");
const Database = require("./src/database.js");

const app = express();
app.use(bodyParser.json());

const PORT = config.PORT;

app.use("/api/", Routes());

new Database(config.MONGODB_URI) // connects to the database using MONGODB cluster URL
.then(() => {
    app.listen(config.PORT, () => {
        console.log(`Server running on port ${config.PORT}`);
    }); 
})
.catch((err) => console.error(err));

/src/routes.js

const express = require("express");

const usersRouter = require("./user/routes.js");


const Routes = (dependencies) =>
{
    const router = express.Router();
    router.use("/users", usersRouter(dependencies));
    return router;
};

module.exports = Routes;

/src/database.js

const mongoose = require("mongoose");

module.exports = class Database{
    constructor(connection){
        this.connection = connection;

        return mongoose.connect(this.connection, {
            useNewUrlParser: true,
            useUnifiedTopology: true
        });
    }
};

src/user/routes.js

const express = require("express");
const UserController = require("./controller.js");
const UserModal = require("./data_access/modal.js");
const UserRepository = require("./repository.js");

const userRoutes = () => {
    const modal = UserModal; // pretty much the User modal/document
    const repository = new UserRepository(modal); // talks to the db
    const router = express.Router();
    const controller = UserController(repository); // handles request, sends repository to services

    router.route('/')
    .get(controller.getUsers)
    .post(controller.addUser);

    return router;
};

module.exports = userRoutes;

src/user/repository.js

module.exports = class UserRepository{
    constructor(model){
        this.model = model;
    }

    create(user){
        return new Promise((resolve, reject) =>{
            this.model(user).save();
            resolve(user);
        });
    }

    getByEmail(email){
        return new Promise((resolve, reject) =>{
            this.model.find({email: email})
            .then((user) => resolve(user[0]));
        });
    }

    getByUsername(username){
        return new Promise((resolve, reject) =>{
            this.model.find({username: username})
            .then((user) => resolve(user[0]));
        });
    }

    getAll(){
        return new Promise((resolve, reject) => {
            const students = this.model.find({});
            resolve(students);
        });
    }
};

src/user/controller.js

const GetUsers = require("./services/get_users.js");
const AddUser = require("./services/add_user.js");

module.exports = (repository) => {
    const getUsers = (req, res) => {
        GetUsers(repository)
        .execute()
        .then((result) => res.sendStatus(200).json(result))
        .catch((err) => console.error(err));
    };

    const addUser = (req, res) => {
        const { username, email } = req.body;

        AddUser(repository)
        .execute(username, email)
        .then((result) => res.send(200))
        .catch((err) => {
            res.send(403);
            console.error(err);
        });
    };

    return {
        getUsers,
        addUser
    };
};

/src/user/services/add_user.js

module.exports = (repository) => 
{
    async function execute(username, email){
        return Promise.all([repository.getByUsername(username), repository.getByEmail(email)])
        .then((user) => {
            if(user[0]){
                return Promise.reject("username already taken!");
            }
            else if(user[1]){
                return Promise.reject("email already taken!");
            }
            else{
                repository.create({username: username, email: email});

                return Promise.resolve("all good!");
            }
        });
    }

    return {execute};
};

src/user/services/get_user.js

module.exports = (repository) => 
{
    async function execute(){
        const users = repository.getAll();
        return new Promise((resolve, reject) => resolve(users));
    }

    return {execute};
};

src/user/data_access/schema.js (entities)

const mongoose = require("mongoose");

module.exports = new mongoose.Schema(
{
    username: {
        type: String
    },
    email: {
        type: String
    },
    password_hash: {
        type: String
    }
});

src/user/data_access/model.js (entities)

const mongoose = require("mongoose");
const userSchema = require("./schema.js");

const UserModal = mongoose.model("User", userSchema);

module.exports = UserModal;
\$\endgroup\$
2
  • \$\begingroup\$ I dont think you need a separate service for get and add. And btw do you really let the modal set password hash? :) \$\endgroup\$
    – slepic
    May 25, 2020 at 8:12
  • 1
    \$\begingroup\$ The current question title of your question is too generic to be helpful. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    May 25, 2020 at 8:21

1 Answer 1

4
\$\begingroup\$

Your code looks very structured and nicely written, but to my understanding, your solution is not "clean architecture" (CA) as described by uncle bob. Your solution is an MVC solution.

In clean architecture you can (for example):

  • easily replace express with another framework
  • easily replace mongo with another DB

In your case the framework is embedded into your logic, you can see that the controller is using res to output the JSON outside. so replacing the framework will require a rewrite.

Another example is that you have no clear boundaries between all the application layers. It can be easily seen that all your imports are from express => to logic => to database and back and that's not using dependency inversion. In CA you have entities layer which is the higher layer and all other layers import the entities as a dependency and it is the communication protocol between all layers.

Other things:

Where is the exception handling Where is the validation handling Where are the multiple ways to output your data (XML, JSON, CSV)

I think you managed to reach a working positive solution (which I think is not extensible for all future cases):

  1. Because you don't have all the required functionality in place (positive and negative).
  2. You don't have unit tests to prove that you are testable
  3. You didn't process your output, there are many HTTP Responses that your presenter needs to handle, but so far you created some basic functionality, so it works.
  4. Where the complexity (as number of API will grow, and when the complexity of the logic will grow) you will start feeling the pain of extensibility.
\$\endgroup\$
5
  • \$\begingroup\$ dude, thank you so much for your answer! indeed, I can see it's not exactly Clean Architecture; i've just started learning software engineering and it's a whole new world. you seem to be knowledgeable and I would love to chat with your for some quick questions if possible. either way, thank you so much. \$\endgroup\$
    – kibe
    Jun 4, 2020 at 5:39
  • 1
    \$\begingroup\$ If that is the case then I would recommend starting with TDD rather then clean architecture. 1) Read TDD by Example of Kent Beck. 2) Please ask your questions \$\endgroup\$
    – Adi Cohen
    Jun 4, 2020 at 8:07
  • \$\begingroup\$ will definitely read the book. I am confused with the "dependency" issue. for example, the service/use-case needs the repository (which talks to the database), correct? should the service get the repository simply by importing it, or should the repository be given to the service in the parameter? thank you \$\endgroup\$
    – kibe
    Jun 4, 2020 at 21:46
  • \$\begingroup\$ You should read about boundaries in the book. Dependency inversion & dependency injection are two things: one is used decoupling layers you don't need to know about, like business logic and DB, needs not to know about each other and you use and adapter pattern to split them using the interface as protocol middleman. The other used to pass instances of object you want to use in a component instead of creating them inline for decoupling and testability \$\endgroup\$
    – Adi Cohen
    Jun 6, 2020 at 20:40
  • \$\begingroup\$ @AdiCohen, can you please indicate what chapter (or pages) in the book discuss boundaries? I tried to find variations of the words "boundary", "entity", "dependency" in the book, but couldn't find what you referred to :( Also, if you know of a good example for a Node.js clean architecture example (e.g., some Github repo) which uses Express and Mongoose as the decoupled example frameworks, it would be great to get a link. \$\endgroup\$ Jan 3, 2021 at 23:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.