Skip to content

Chitter Challenge Pull Request - #2196

Open
aubreysalmins wants to merge 28 commits into
makersacademy:mainfrom
aubreysalmins:main
Open

Chitter Challenge Pull Request#2196
aubreysalmins wants to merge 28 commits into
makersacademy:mainfrom
aubreysalmins:main

Conversation

@aubreysalmins

Copy link
Copy Markdown

I hope I've done this pull request right! :/

Comment thread app.rb
peep.title = params[:title]
peep.content = params[:content]
peep.time_stamp = Time.new
peep.user_id = 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to tidy this up a bit more - by tidying, I mean moving some of the 'business logic' out of the app.rb - you could write something like peep = Peep.new(params[:title], params[:content], etc., passing in these parameters as arguments to a new Peep object, and storing the variables that way instead.

The benefit of doing this is that you push some of the responsibility to the Peep class instead of the app.rb. This will adhere more closely to 'Single Responsibility Principle', which guides us to try to give each class one overarching responsibility.

In the context of a full stack web app, the pattern we follow is Model View Controller, or MVC. In this case, app.rb is the Controller - it's responsible for responding to URL requests / button actions etc. and deciding which code to run - the Models are responsible for structuring our objects like Peeps and Users, and communicating with the database through the Repository classes. And the Views are responsible for what's visible to the user.

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