Created
June 1, 2015 21:14
-
-
Save panchew/8beefe8f5ece6b247343 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Client::PortalController analysis. | |
What does it do? | |
It seems to take care of the requests related to proposals. It has an action that takes care of both showing and previewing a proposal (the type of viewing gets chosen in #info action. It also has #index but the way the controller is implemented doesn't seemt to be RESTful (not a resource in the routes file). | |
I can see there is logic enable via includes that could enable the 'presenter' design pattern, and in the #canvas action, I can see a bit of the 'template' design pattern, and also I can see there are calls to Amazon S3 service; also the proposals states and acceptance are handled by this controller as well as comments related to a single proposal. I noticed mailer logic for several actions: acceptance, set_status and show (although I can't see why ClientEmailer.deliver_proposal_accepted(proposal) is called in two different actions. | |
What I like about it. | |
- I can distinguish a bit of design patterns being applied (though I'm not an expert using them) and re-using code from external files. | |
- The code is understandable (not super self-explanatory, but clean) | |
- The correct use of namespaces. | |
What I don't like about it. | |
- Some actions (methods) are long and seem to be good candidates for a refactoring | |
- The controller seems to have its purposes related to proposals, and maybe it should be done in a RESTful way by moving the logic to a ProposalsController |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment