Open longnd opened 2 years ago
The Keyword model is handling too many responsibilities: it acts as a model but also handles the queries and the scrapping work at the same time.
Besides that, putting the keyword upload limit (20) into the model doesn't make much sense. The naming isn't descriptive either. https://github.com/ltenzil/scrape/blob/6eb163261a9102f748fc789147840f7856584ef8/app/models/keyword.rb#L8
This file should be refactored to organize the code by business logic, e.g.
KeywordQuery
Similar to that, the KeywordController#bulk_upload method is also doing multiple things at once: parsing the uploaded file, doing the validation, calling the scrapping work. Why not delegate the CSV parsing & validation to other methods? https://github.com/ltenzil/scrape/blob/6eb163261a9102f748fc789147840f7856584ef8/app/controllers/keywords_controller.rb#L73-L90
KeywordController#bulk_upload
Sure long, will refactor it. Will use service objects (KeywordQuery or CSV validation) over model.
The Keyword model is handling too many responsibilities: it acts as a model but also handles the queries and the scrapping work at the same time.
Besides that, putting the keyword upload limit (20) into the model doesn't make much sense. The naming isn't descriptive either. https://github.com/ltenzil/scrape/blob/6eb163261a9102f748fc789147840f7856584ef8/app/models/keyword.rb#L8
This file should be refactored to organize the code by business logic, e.g.
KeywordQuery
for exampleSimilar to that, the
KeywordController#bulk_upload
method is also doing multiple things at once: parsing the uploaded file, doing the validation, calling the scrapping work. Why not delegate the CSV parsing & validation to other methods? https://github.com/ltenzil/scrape/blob/6eb163261a9102f748fc789147840f7856584ef8/app/controllers/keywords_controller.rb#L73-L90