Skip to content

Instantly share code, notes, and snippets.

@rails-hub
Last active June 16, 2017 17:43
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save rails-hub/e394914775a3ca8e94641ae74e795752 to your computer and use it in GitHub Desktop.
Save rails-hub/e394914775a3ca8e94641ae74e795752 to your computer and use it in GitHub Desktop.
A. Annotate the Code Above with an explanation of what's being done
One liner answer:
Its finding the missions with eager loading the milestones, company etc after applying the filters depending upon what filters(search/mission type id) has been passed to it.
B. List the params being acted upon
1. mission_type_id
2. search
C. List the variables being exposed for the view and what you expect they'll contain
1. @mission_types
- Contains the missions types for the current companies's missions.
2. @mission_type
- Contains an instance of Mission Type depending upon what filter has been provided, it the mission_type_id filter is present then it will fetch the specific one else it would go for the first one from the mission types active record relation.
3. @missions
- Contains the missions.
4. @featured_present
- Contains boolean value.
D. Annotate or propose how you'd improve the code for readability/maintainability
Honestly speaking this is quite messed up. Here is what I would have preferred.
1. I won't be using any instance variable which is actually against the idea of encapsulation.
2. There is a lot of code which should have been located in the respective models within the instance methods(we may look the first variable mission_type_ids).
3. To me @mission_type, @mission_types and @featured_present instance variable look quite redundant here(I'm not sure what's the use of them in the views, to me they are pretty much uncalled for).
4. There should be no code here which is specific for the presenters and decoraters so I feel all this controller could just have been 3-4 lines of code at max and the decoration could have been moved in the MissonsDecorator which could have made the controller quite simpler/readable and maintainable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment