Created
February 14, 2017 16:40
-
-
Save davetron5000/52bd0ee50ff16e4a33b356ceb84c01cb to your computer and use it in GitHub Desktop.
DRY Madness
This file contains 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
# This is DRY run amok. There is no reason to create an abstraction around | |
# loading all the notes when we have it already: Note.all | |
# The private methods add no value. | |
class NotesController < ApplicationController | |
def index | |
load_notes | |
end | |
def show | |
load_note | |
end | |
def new | |
build_note | |
end | |
def create | |
build_note | |
save_note or render 'new' | |
end | |
def edit | |
load_note | |
build_note | |
end | |
def update | |
load_note | |
build_note | |
save_note or render 'edit' | |
end | |
def destroy | |
load_note @note.destroy redirect_to notes_path | |
end | |
private | |
def load_notes | |
@notes ||= note_scope.to_a | |
end | |
def load_note | |
@note ||= note_scope.find(params[:id]) | |
end | |
def build_note | |
@note ||= note_scope.build @note.attributes = note_params | |
end | |
def save_note | |
if @note.save | |
redirect_to @note | |
end | |
end | |
def note_params | |
note_params = params[:note] | |
note_params ? note_params.permit(:title, :text, :published) : {} | |
end | |
def note_scope | |
Note.all | |
end | |
end |
This file contains 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
# This is with no private methods and just the vanilla "Rails Way" | |
# There is some duplication (Note.find(params[:id])), but TBH is that | |
# likely to ever change? The entire point of Rails and Active Record is | |
# the way you locate an object is by looking it up by ID, and Rails | |
# makes a very strong guarantee that that ID is in params as :id. | |
# | |
# Further, what is gained by extracting if @note.save; redirect_to @note into | |
# a private method? It's two lines of code and IMO, it's much easier to just | |
# have those two lines inlined so you can quickly see what the method does. | |
# Creating a "save_note" abstraction is needless complexity for no real gain. | |
class NotesController < ApplicationController | |
def index | |
@notes = Note.all | |
end | |
def show | |
@note = Note.find(params[:id]) | |
end | |
def new | |
@note = Note.new | |
end | |
def create | |
@note = Note.new(note_params) | |
if @note.save | |
redirect_to @note | |
else | |
render 'new' | |
end | |
end | |
def edit | |
@note = Note.find(params[:id]) | |
end | |
def update | |
@note = Note.find(params[:id]) | |
@note.udpate(note_params) | |
if @note.save | |
redirect_to @note | |
else | |
render 'edit' | |
end | |
end | |
def destroy | |
@note = Note.find(params[:id]) | |
@note.destroy | |
redirect_to notes_path | |
end | |
end |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Its gets more useful when you have to do something interesting with the build_note (like maybe create some dependent objects) or you need to do some kind of filtering (current_user.notes).