📅 October 02, 2019
👷 Chris Power
I think it’s pretty fair to say that as we grow older and wiser, we tend to forget where we came from. And I think this recency-bias shows up frequently in software engineering as well. In software engineering, you tend to build on layers of layers of skills that you accumulate over a long period of time. After being a programmer for, lets say 10 years, you may forget how much you struggled early in the process. As you learn more advanced techniques, you tend to forget how much you may have struggled with even the simplest of programming concepts. I even find myself losing patience with new engineers from time to time, wondering how they couldn’t understand some basic techniques that I take for granted every day.
I think this is a form of hubris — excessive pride or self-confidence. As we grow more experienced in a given field, we gain more and more hubris. We tend to forget from where we started, and feel more confident in the skills we’ve acquired. Some of you may think “But Chris! I struggle with imposter syndrome all the time!”, and I agree with you (I do as well). Software Engineering is a very humbling field where one cannot literally learn enough. However, if you had 10+ years of experience and I asked you to write out a simple “fizzbuzz” problem on a whiteboard, you’d scoff at it wouldn’t you? (you know you would).
The point I’m trying to make is that some people are just starting out in their programming careers. And they may struggle with basic algorithms, fizzbuzz-type challenges, and — as we’ll see in a moment — writing a simple controller in Ruby on Rails. I think its easy to forget where you came from, because I know I have.
if you had 10+ years of
experience and I asked you to write out a simple “fizzbuzz” problem on a whiteboard, you’d scoff at it wouldn’t you? (you know you would).
Recently, I received a security vulnerability alert for one of my private repositories in github. I recognized the name of the project, but I don’t remember the last time I opened the codebase. The vulnerability alert came from a repository called “REAPP” (it stood for real estate application).
This application was my first attempt to build a Property Management platform — a SaaS product that allows landlords/property managers to accept rent, manage tenants, and more. I have since re-written this application many times over the last 10 years or so, but this was it’s first iteration. Clicking around in the codebase, I remembered how hard I worked to get this code to do much of anything. Specifically, I can remember a “properties” controller I wrote, and the struggle I had with it. I am pretty sure this controller made me abandon the project, because it was just too hard of a problem to solve.
Let me just paste the code below, for you all to enjoy. Continue below the fold for my analysis and thoughts.
# GET /properties/1
# GET /properties/1.xml
def show
@property = Property.find(params[:id])
#Handing tenants
@tenants = @property.tenants.all
if @tenants.empty?
flash.now[:success] = "Weclome to LivingRoom!\r\n
We noticed you don't have any tenants set up, please find the 'tenants' box on the right hand side of your screen
and be sure to add your first tenant before anything else.
Thanks!"
end
#handling incomes, and the income chart series data
@incomes_all = @property.incomes.all
@incomes_full = @property.incomes
@incomes = @property.incomes.paginate(:page => params[:income_page], :per_page => 5)
@incomes_sum = @incomes_all.sum(&:income_amount)
@income_chart_monthly = incomes_chart_series_monthly(@incomes_full, 4.months.ago)
#handinling expenses and the expenses chart series data
@expenses_all = @property.expenses.all
@expenses_full = @property.expenses
@expenses = @property.expenses.paginate(:page => params[:expense_page], :per_page => 5)
@expenses_sum = @expenses_all.sum(&:expense_value)
@expense_chart_monthly = expenses_chart_series_monthly(@expenses_full, 4.months.ago)
@net_series = net_series_monthly()
#zillow chart stuff
@zillow_chart_url = zillow_chart()
@valuation = zillow_value()
end
def zillow_chart
@rillow = Rillow.new("MY-ZILLOW-CREDS")
@zillow_search = @rillow.get_search_results(@property.street_address,
@property.city+","+ @property.state)
@zillow_property_id = @zillow_search.find_attribute("zpid")
@zillow_property_id = @zillow_property_id.join
@z_chart = @rillow.get_chart(@zillow_property_id, "percent", :width => 200, :height => 130, :chart_duration => "5years")
@result_url = @z_chart.find_attribute("url")
return @result_url.join
end
def zillow_value
@rillow = Rillow.new("MY-ZILLOW-CREDS")
@zillow_search = @rillow.get_search_results(@property.street_address,
@property.city+","+ @property.state)
@zillow_search = @zillow_search.to_hash
@value = @zillow_search.find_attribute("valuationRange")
return @value
end
# GET /properties/1
# GET /properties/1.xml
def show_expenses
@property = Property.find(params[:id])
@tenants = @property.tenants.all
@expenses_full = @property.expenses
@expenses = @expenses_full.paginate(:page => params[:page], :per_page => 10)
@expense_chart_monthly = expenses_chart_series_monthly(@expenses_full, 4.months.ago)
end
# GET /properties/1
# GET /properties/1.xml
def show_incomes
@property = Property.find(params[:id])
@tenants = @property.tenants.all
@incomes_full = @property.incomes
@incomes = @incomes_full.paginate(:page => params[:page], :per_page => 10)
@income_chart_monthly = incomes_chart_series_monthly(@incomes_full, 4.months.ago)
end
# GET /properties/1
# GET /properties/1.xml
def show_tenants
@property = Property.find(params[:id])
respond_to do |format|
format.html # show.html.erb
format.xml { render :xml => @property }
end
end
I don’t think I knew what the difference was between an instance variable and a regular variable. I definitely felt the need to make almost everything an instance variable, regardless of my understanding.
I pasted the show
action in this controller for a reason — its god
awful. I’m not sure why I needed 5 different instance variables to represent
incomes:
@incomes_all = @property.incomes.all
@incomes_full = @property.incomes
@incomes = @property.incomes.paginate(:page => params[:income_page], :per_page => 5)
@incomes_sum = @incomes_all.sum(&:income_amount)
@income_chart_monthly = incomes_chart_series_monthly(@incomes_full, 4.months.ago)
I most likely just wanted to use the paginated incomes for a property, not all of the incomes. From there, I could have pulled the chart data, and had it update when a user changed the query param for page. That would have been nice.
I like this part of the controller:
@zillow_chart_url = zillow_chart()
@valuation = zillow_value()
First of all, these names make no sense. Why would I make a method named
zillow_chart
that returns a url? These, and other methods, should have been
placed in their own service objects to encapsulate different behaviors and
concerns.
This was a fun trip down memory lane. But I think some lessons can be learned here. Whenever I find myself frustrated reviewing a junior developer’s code, I should remember that I once wrote an insane controller with 40+ instance variables, no separation of concerns, and — oh yeah — INLINE API CREDENTIALS. I am also pretty sure this was completely untested. Actually let me check … yup, no tests whatsoever. 😊
We're trusted by large, medium, and small companies all over the world
Have something you're working on?
Tell Us About It