Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
DRY Madness
# 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 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
@johnpaulashenfelter

This comment has been minimized.

Copy link

johnpaulashenfelter commented Feb 14, 2017

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.