• Grab a piece of paper.
• Grab a pen if you need one.

Overview

• Preliminaries.
  • Admin.
  • Upcoming Work.
  • Extra Credit.
  • Questions.
• An Exercise.
• Legacy code.
• Smells and refactoring.
• Some examples.

Preliminaries

Admin

• Wednesday's class will be on UML. Albert will lead class.
• Prof. Curtsinger will be teaching Friday's class. Please send your journal reflections to both of us.
• Missing: AM, BL

Upcoming Work

• Readings for Friday
  • Arvind Narayaan and Shannon Vallor (March 2014). Why software engineering courses should include ethics coverage. Communications of the ACM 57(3): 23-25. http://dl.acm.org/citation.cfm?id=2566966
  • Neil McBride (August 2012). The ethics of software engineering should be an ethics for the client. Communications of the ACM 55(8): 39-41. http://dl.acm.org/citation.cfm?id=2240250
  • Deborah G. Johnson (October 2008). Computer experts: Guns-for-hire or professionals? Communications of the ACM 51(10): 24-26. http://dl.acm.org/citation.cfm?id=1400190
• Reading Journal for Friday (due Thursday at 8pm)
  • Subject: Reading Journal for Class 18 (Your Name)
  • From each of the three new articles about software engineering ethics, select one sentence that you think best captured its thesis. Quote that sentence, giving a page number and approximate location on the page (e.g., "top right").
  • What is one question arising from these articles that you'd like to discuss during class?
  • Brainstorm a few topics or questions that you might address in a 600-word essay on software engineering ethics (the last assignment). Your ideas might arise from the articles or from current events.
  • As is often the case, these questions were inspired by the work of JLND.

Good Things to Do

Academic

• MET Opera Saturday.

Peer

• Christine's show!
• Toby's Pub Talk on Plans on Thursday

Other

• Vote today!

Questions

Preparatory Activity

On a sheet of paper (supplied), answer the following questions.

1. What is "legacy code"? (We see lots of definitions within the chapter.)
2. List three "code smells" that you regularly notice.
3. What is SOFA?
4. List three refactoring techniques that you regularly use.

Legacy Code

• "Old" code. Written before you have a chance to work on it.
• Implemented already.
• Probably needs to be updated or maintained.
• Still in use.
• Usually poorly documented.
• Sometimes only runs on certain machines.

Approaches to handling legacy code?

• Read and document.
• Work with another programmer who knows the code.
• Write tests to understand current behavior.
• Refactor to clarify.

Code Smells

• Lexicon of common ways that code may be problematic.
  • Terms to share.
  • Checklist for others to apply.
• SOFA - code that generally smells good
  • S - short
  • O - one purpose
  • F - few parameters
  • A - appropriately abstract
• We all have things that signal a possible problem.
  • Unclear names - harder to understand; may mean that changes will break things.
  • Long, long enough that you can no longer easily track the flow of control. (Possible hidden thing that is rarely reached.)
  • Multiple purposes - Optimize or fix one purpose may damage another; usually more complex
  • Multiple parameters - Easy to get wrong; shows lack of encapsulations
  • Wrong level of abstraction - Bigger picture; not thining how the parts relate.
  • Nearly identical code (usually from copy paste); not DRY. Makes upgrades more difficult.
  • High "ABC" score - Lots of assignments, branches, and conditionals
  • Hard-coded parts of the program (e.g., char str[20] or if (strcmp(passwd,"hello world) == 0)). Can be hard to fix. Not general, and you will probably need it generalized eventually.
  • Cascading dependencies - A change in one place will force a change in many others.
  • Failing to follow the organization's style (e.g., formatting, naming, etc.). Harder to read; someone else may get confused. Manager is slacking. People who don't follow simple rules also tend to ignore other rules. (Style includes indentation rules and whether you use space or tabs or ...)
  • Nearly identical code at multiple points in the same procedure. (Code with similar structures, but at a more fine-grained level; same objections.)
  • Classes that only delegate? (Actually, a good genre of classes.) Seem wasteful.
  • Obscure comments and poor documentation.
  • Convoluted conditionals - Hard to track execution.
  • Large classes, with lots of methods. Might want fewer classes that you then group together. Can improve complex dependencies.
  • Strange inter-class calls.
  • Functions that replace a single line of code. (May add clarity, but at the cost of a function call.) (May not add clarity.)
  • Conditionals the check for similar things.
  • Magic numbers.
  • Comments denoting multiple phases of a function.

• Other likely problems that may be harder to note

  • Code that is never executed

    x = false; if (x) { ... }

Refactoring

• Move things to an array or list and loop through them.

An 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

We will focus on

      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

What smells bad?

• Long method. Write helpers or something similar. Does too many things.
• Repeated code. (Increasing amount paid, etc.)
• Magic numbers.
• Real numbers for money. Bad bad bad.
• Peeking into another class. Maybe movies should figure out how much they cost.
  • Could have a field for class.
  • Could subclass

Solutions

• HashMaps