Software Development (CSC 321 2015F) : EBoards

[Skip to Body]

Primary: [Front Door] [Schedule] - [Academic Honesty] [Disabilities] [Email] [FAQ] [Teaching & Learning]

Current: [Outline] [EBoard]

Sections: [Assignments] [EBoards] [Examples] [Handouts] [Outlines]

Reference: [Slack] [Ruby@CodeCademy]

Related Courses: [CSC 321 2015F @ EdX] [Berkeley CS 169 2014F] [CSC 321 2015S (Davis)] [CSC 322 2015F (Rebelsky)]

Misc: [SamR] [Glimmer Labs] [CS@Grinnell] [Grinnell] [Issue Tracker]

CSC 321 2015F, Class 12: Legacy Code, Refactoring, and Beyond

Overview

Preliminaries

Admin

Questions

Legacy Code

Preliminary Questions

What is legacy code? - Some definitions

What is legacy code? - Some questions

Additional questions -

Why do we care about legacy code?

In what ways do we "maintain" legacy code?

How do we do this kind of maintainance?

Refactoring

What is refactoring?

What are some techniques?

Other issues

Sam's critique/commentary

What are your common techniques for improving your own code? (Yes, they could match the ones that they suggest in the book.)

Questions

Detour on Old-School Computerized Language Translation

An Example

class TimeSetter
  def self.convert(d)
    y = 1980
    while (d > 365) do
      if ((y % 400 == 0) || (y % 4 == 0) && (y % 100 != 0))
        if (d > 366)
          d -= 366
          y += 1
        end
      else
        d -= 365
        y += 1
      end
    end
    return y
  end
end

Our book refactors this to the following code.

class TimeSetter
  ORIGIN_YEAR = 1980
  def self.calculate_current_year(days_since_origin)
    @@year = ORIGIN_YEAR
    @@days_remaining = days_since_origin
    while (@@days_remaining > 365) do
      if leap_year?
        peel_off_leap_year!
      else
        peel_off_regular_year!
      end
    end
    return @@year
  end

  private
  def self.peel_off_leap_year!
    if (@@days_remaining >= 366)
      @@days_remaining -= 366
      @@year += 1
    end
  end
  def self.peel_off_regular_year!
    @days_remaining -= 365;
    @year += 1;
  end
  def leap_year?
    @year % 400 == 0 ||
      (@year % 4 == 0 && @year % 100 != 0)
  end
end

In contrast, we rewrote (refactored?) it to the following code.

lass TimeSetter
  def self.isLeapYear(year)
    ((year % 400) == 0) || ((year % 4 == 0) && (year % 100 != 0))
  end

  def self.daysInYear(year)
    if isLeapYear(year)
      366
    else
      365
    end
  end

  def self.convert(d)
    y = 1980;
    while (d > daysInYear(y))
      d -= daysInYear(y)
      y += 1
    end
    return y
  end
end

What do you see as the strengths and weaknesses of each strategy?

Another Example

From Field et al., pp. 2-4.

class Movie
  REGULAR = 0
  NEW_RELEASE = 1
  CHILDRENS = 2
  attr_reader :title
  attr_accessor :price_code
  def initialize(title, price_code)
    @title, @price_code = title, price_code
  end 
end

class Rental
  attr_reader :movie, :days_rented
  def initialize(movie, days_rented)
    @movie, @days_rented = movie, days_rented
  end 
end

class Customer
  attr_reader :name

  def initialize(name)
    @name = name
    @rentals = []
  end

  def add_rental(arg)
    @rentals << arg
  end

  def statement
    total_amount, frequent_renter_points = 0, 0
    result = "Rental Record for #{@name}\n"
    @rentals.each do |element|
      this_amount = 0

      # determine amounts for each line
      case element.movie.price_code
      when Movie::REGULAR
        this_amount += 2
        this_amount += (element.days_rented - 2) * 1.5 if element.days_rented > 2
      when Movie::NEW_RELEASE
        this_amount += element.days_rented * 3
      when Movie::CHILDRENS
        this_amount += 1.5
        this_amount += (element.days_rented - 3) * 1.5 if element.days_rented > 3
      end

      # add frequent renter points
      frequent_renter_points += 1
      # add bonus for a two day new release rental
      if element.movie.price_code == Movie.NEW_RELEASE && element.days_rented > 1
          frequent_renter_points += 1
      end

      # show figures for this rental
      result += "\t" + element.movie.title + "\t" + this_amount.to_s + "\n"
      total_amount += this_amount
    end

    # add footer lines
    result += "Amount owed is #{total_amount}\n"
    result += "You earned #{frequent_renter_points} frequent renter points"
    result
  end
end

Our improvements.

class Movie
  REGULAR = :regular
  NEW_RELEASE = :new_release
  CHILDRENS = :childrens
  attr_reader :title
  attr_accessor :price_code
  def initialize(title, price_code)
    @title, @price_code = title, price_code
  end 
end

class Rental
  attr_reader :movie, :days_rented
  def initialize(movie, days_rented)
    @movie, @days_rented = movie, days_rented
  end 
  def price
    rental_price = 0
    case element.movie.price_code
    when Movie::REGULAR
      rental_price += 2
      rental_price += (element.days_rented - 2) * 1.5 if element.days_rented > 2
    when Movie::NEW_RELEASE
      rental_price += element.days_rented * 3
    when Movie::CHILDRENS
      rental_price += 1.5
      rental_price += (element.days_rented - 3) * 1.5 if element.days_rented > 3
    end
    rental_price
  end

  def points
    if element.movie.price_code == Movie.NEW_RELEASE && element.days_rented > 1
      2
    else
      1
    end
  end
end

class Customer
  attr_reader :name

  def initialize(name)
    @name = name
    @rentals = []
  end

  def add_rental(arg)
    @rentals << arg
  end

  def statement
    result = "Rental Record for #{@name}\n"
    @rentals.each do |element|
      this_amount = 0

      # determine amounts for each line
      this.amount = element.price

      # show figures for this rental
      result += "\t" + element.movie.title + "\t" + this_amount.to_s + "\n"
    end

    # add footer lines
    result += "Amount owed is #{total_amount}\n"
    result += "You earned #{frequent_renter_points} frequent renter points"
    result
  end

  def frequent_renter_points
    @rentals.inject(0) { |result,element| result + element.points }
  end

  def total_amount
    @rentals.inject(0) { |result,element| result + element.amount }
  end

end