The controller BE/src/modules/search/search.controller.ts and the service BE/src/modules/search/search.service.ts are handling too many responsibilities:
Controller:
Not expected ❌
Parsing the CSV file
Handling the strategy to parallelize each scraping job
Parsing the scrapping result into Payload models
DB persistence
Expected ✅
request params validation
middleware for authentication
error handling
response handling
Service
Google Search HTTP requests
HTML parsing (to find the results, link count, etc...)
This has impacts on the maintainability, readability, and testability of the code.
For example, if the service has both the Google HTTP request and the HTML parsing, it makes it much harder to write tests for the HTML parsing (because you will have to mock the tool used for HTTP requests). And if you need to change the tool for HTTP requests later, you will have to re-write your HTML parsing tests.
Using two separate classes (or functions) allows for a better separation of concerns.
dtvala2930
commented
3 months ago
Issue
The controller
BE/src/modules/search/search.controller.tsand the serviceBE/src/modules/search/search.service.tsare handling too many responsibilities:Controller:
Service
This has impacts on the maintainability, readability, and testability of the code.
Expected
The code is expected to follow the Single Responsibility Principle as much as possible.
For example, if the service has both the Google HTTP request and the HTML parsing, it makes it much harder to write tests for the HTML parsing (because you will have to mock the tool used for HTTP requests). And if you need to change the tool for HTTP requests later, you will have to re-write your HTML parsing tests.
Using two separate classes (or functions) allows for a better separation of concerns.