Skip to content

Decoupling controller#18

Open
Onesco wants to merge 6 commits intoanomic30:mainfrom
Onesco:decoupling_controller
Open

Decoupling controller#18
Onesco wants to merge 6 commits intoanomic30:mainfrom
Onesco:decoupling_controller

Conversation

@Onesco
Copy link
Contributor

@Onesco Onesco commented Sep 26, 2022

Description

Added the readFileSync method, fixed the User.findOne().select() error.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Onesco Onesco marked this pull request as draft September 26, 2022 10:50
@ezehlivinus
Copy link
Contributor

@Onesco why are the controllers' methods all statics methods?

@Onesco Onesco marked this pull request as ready for review October 3, 2022 06:01
@Onesco
Copy link
Contributor Author

Onesco commented Oct 3, 2022

@ezehlivinus. The controllers method are static method so that you can call them without having anyway within your code witgout having to create an Instance of the Object. Since it was meant to replace the Original Mongoose methods there is no need getthing to instantiate the Metthod Object before calling the method so to mentain consistency across board need to use static method was intuitive to me. Although if you have any other view i am open to hear and learn to. Thanks for asking!

@ezehlivinus
Copy link
Contributor

ezehlivinus commented Oct 3, 2022

@Onesco I understand. Static method are used so that one can call the method directly. This is how I do it and it is popular among express developers. Between what you used and this below I don't know which have more advantage.

class UserController {
 create () { }
}

//
module.exports = new UserController ()

So, when importing into the route files it is an instance of it that get import.

//es6 module
import userController from '../controller/user.controller.js'

// Or commonJS
const userController = '../controller/user.controller.js'

router.post('/', userController.create )

But I think we don't need to make static methods at this point, if so we could have used plain Object to define the functions.

module.exports = {
 create() { }
}

@ezehlivinus. The controllers method are static method so that you can call them without having anyway within your code witgout having to create an Instance of the Object. Since it was meant to replace the Original Mongoose methods there is no need getthing to instantiate the Metthod Object before calling the method so to mentain consistency across board need to use static method was intuitive to me. Although if you have any other view i am open to hear and learn to. Thanks for asking!

@Onesco
Copy link
Contributor Author

Onesco commented Oct 3, 2022

@ezehlivinus you are right either ways are used...the only difference here is that we have no need for the compiler to instantiate an instance of our object. The choice of static method over regular method for the ontrollers is just to avoid creating an instance of the controller object and eliminating the likelihood of one exporting the class instead of the class object.

class UserController {
 create () { }
}

// likely omission
module.exports = UserController ()

instead of this

class UserController {
 create () { }
}

//
module.exports = new UserController ()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants