Skip to main content

CSC 321.01, Class 18: Refactoring code

Overview

  • Preliminaries
    • Notes and news
    • Upcoming work
    • Extra credit
    • Questions
  • Refactoring techniques
  • Refactoring practice

Preliminaries

News and notes

  • Sorry about Wednesday.
  • Many of you seem to be falling behind on homework. Congratulations on re-verifying that I need to give particular questions to keep you moving forward.

Upcoming work

Good things to do (Academic/Artistic)

  • Faulconer gallery has two great exhibits.

Good things to do (Other)

  • Fill out NCHA Survey (aka the latest food truck survey).
    • Students received e-mails from “Jen Jacobsen ncha-web@acha.org” with the subject line “NCHA survey: Help GC help you + food trucks!” (Depending on outlook settings, this may end up in other or clutter folders)
    • NCHA data help me give my PSAs
  • Watch President White dive (I’m not sure where the live stream is)
  • Support GHS Girls Basketball

Questions

Refactoring techniques

Think -> Pair -> Share: What are your favorite refactoring techniques? (These can be from the book or from elsewhere)

  • Decompose conditionals - Instead of one big conditional, write separate methods for the different parts.
  • Extract methods: Look for chunks of code that might be more sensibly made into a method.
    • E.g., reusing similar code.
    • Sam notes that you don’t always write a new method to deal with similar code; there are other approaches, too.
  • Make the code clearer so that when you come back to it its clear(er), or at least less murky.
    • Good variable and method and object names
    • Add comments - Need to stay in synch with the code
    • Consistent formatting
  • Replace magic numbers with names
    • Magic numbers can be hard to read; it may have meaning to you, not to others.
    • Easier to change magic numbers if you have one place to change them. (define ARRAY_SIZE 10). Sometimes the same number has different meanings.
  • When you have multiple parameters to a procedure, group them into a single object for clarity and convenience. That also allows you to use instance methods and such.
  • Limit responsibility. Each class should have one clear purpose. (Break complex classes into smaller classes that you then group together.)

Refactoring practice

class TimeSetter
  def initialize(d)
    @d = d
  end

  def iso8601_ordinal
    d = @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.to_s + "-" + d.to_s
  end
end

In the world view (umwelt) of this system, dates are stored as a single POSITIVE integer, d, which represents the number of days since the beginning of time (December 31, 1979).

The goal is to translate the number of days since the start of time to a year and the day within that year. E.g., 400 is the 34th day of 1981. A sample quick call is TimeSetter.new(400).iso8601_ordinal

What unit tests would you write for this code?

  • 1 => “1980-1”
  • 366 => “1980-366”
  • 367 => “1981-1”
  • 365+366 => “1981-365”

Reminder: Some problems we identified

  • Magic numbers: 1980, 365, …
  • Duplicated code (d -= X; y += 1). Needs to be DRYer.
  • Complex Boolean expression.
    • Suggested replacement: Use a procedure/predicate.
    • Suggested replacement: Clean up the code, which seems repetitious.
  • Complex nested conditionals.
  • Use of object-oriented design for what is really an imperative method
  • Undocumented! (Code is not necessarily self documenting.)

Note: This code is borken. We’ll fix it today.

# Dates stored in number of days since the epoch (31 December 1979).
# If daysSinceEpoch is 1, this is January 1, 1980.
class TimeSetter
  # The initial year
  @@INITIAL_YEAR = 1980

  def initialize(daysSinceEpoch)
    @daysSinceEpoch = daysSinceEpoch
  end

  def isLeapYear(year)
   ((y % 400 == 0) || (y % 4 == 0) && (y % 100 != 0))
  end

  def daysInYear(year)
    if isLeapYear(year) then
      366
    else
      365
    end
  end

  # Convert the date to is8601 ordinal format (YYYY-DDD, where DDD is the
  # ordinal number of the day within the year)
  def iso8601_ordinal
    daysSinceStartOfYear = @daysSinceEpoch
    currentYear = @@INITIAL_YEAR
    while (daysSinceStartOfYear > daysInYear(currentYear))
      daysSinceStartOfYear -= daysInYear(currentYear)
      currentYear += 1
    end
    return y.to_s + "-" + d.to_s
  end
end

Some morals

  • Some repetitious code gets fixed through other mechanisms.