CSC 321.01, Class 16: Legacy code and code smells

Overview

• Preliminaries
  • Notes and news
  • Upcoming work
  • Good things to do
  • Questions
• Debrief
• Legacy code
• Code smells - An example

Preliminaries

News and notes

• Since I’ve had to miss a few classes, I may hold CSC 321 sessions during week eight. Are there topics that you feel like you’d benefit from increased coverage of?
  • Current answer: No.
• Mentor sessions: Tuesday night 6-7, Thursday 6:30-7:30

Upcoming work

• Chapter 7 of Hartl due Thursday at 10:30 pm.

Good things to do (Academic/Artistic)

• Faulconer gallery has two great exhibits.
• Convocation Thursday.

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

Questions

Debrief from last week: Modeling

Think -> Pair -> Share: What did you learn about modeling?

• Modeling is not necessarily straightforward. There are lots of design choices you can make.
• Although we have taught you to think about modeling in terms of objects or structs, databases and DBMSs are a useful tool to think about modeling. (Plus they get used by businesses.)
• Data modeling organizes our data and allows us to think about relationships.
• Good data modeling results in DRY data. - Data are not repeated at different points in the database.
• Normally, three kinds of relationships: 1:1, 1:many, many:many
  • Rails has something you can write in a model to suggest a relationship. has_many, has_one, …
  • Different strategies for doing this in SQL.

Sam’s comments

• Think about not just how you group data, but also the relationships between the groups/objects you create.
• Keep your design DRY. To reference the same set of data in multiple places, use references (pointers) rather than copying.
• SQL appears as a job requirement in many ads.
  • Even though you don’t realize it, about two days of work will teach you more SQL than you will need for an interview.

Debrief from last week: Testing

Think -> Pair -> Share: What did you learn about testing?

• Unit vs. Integration vs. System
• Test-driven development: Write tests, then code.
  • Extreme version: One test at a time.
• Unit testing - Focuses on small parts of the program (individual methods or objects)
• Integration and system testing - Testing how the whole system works together.
  • Often use integration for smaller combinations and system for the whole integrated blob.
  • Some use integration testing for “All of this must pass before I allow your code into my repo.”

An exercise: How would you test binary search?

Here’s a binary search method.

// Find the index of a within A.
// Produces: i, an integer
// Pre: A is sorted from smallest to largest
// Post: If a appears within A, A[i] = a
// Post: A does not appear within A iff i is -1.
extern int binarySearch(int a, int A[]);

What tests would you write? (English is fine.) (You do not need to worry about what happens if the preconditions are not met.)

for size = 0 to 32
   A = an array of the given size, filled with even numbers 
       from 0 to (size-1)*2
   for a = -1 to (size-1)*2-1
     verify that if a is odd then binarySearch(a,A) = -1
     verify that if a is even then binarySearch(a,A) = a/2

Sam thinks that good testing involves more than just a few examples. Good testing is systematic, and therefore probably involves writing some loops.

Legacy code

“‘As Chapter 1 explained, legacy code stays in use because it still meets a customer need even though its design or implementation may be outdated or poorly understood.’ [F&P 9.1] Chapter 1 says: ‘The term legacy code refers to software that, despite its old age, continues to be used because it meets customer needs. [F&P 1.7].’” [I appreciate the direct quotations. I would like to see things in your own words.]

“Even after reading the chapter I feel that the term “legacy code” is ambiguous in written meaning but most people can see and feel if a given codebase should be considered “legacy”. In terms of definitions, I don’t have a concrete one but I would say that most legacy code has one or more of the following properties:- (a) Written by someone else or a team that is no longer working on the project (b) Written in a version of a language which is not considered deprecated or obsolete (eg. JTSE 1.3) (c) Due to continual changes, the well-engineered architecture has transformed into a monstrous mash of patches over the original application.” [Yay for intro comment and for multiple models.]

“Legacy code is code that is no longer supported it be due to advances in hardware and or software.” [Questionable definition; questionable grammar.]

“Still in use because it meets a customer need (though the code or documentation could be outdated). Section 9.4 also empathizes the importance of good commenting in legacy code, and how it may not exist, and when you inherit this code, it is your job to go back and comment correctly. Legacy code may not even be outdated! It could simply just be code that a programmer inherits, which need organization or functional improvements.” [Nongrammatical start, good final sentence.]

“Legacy code is code that lacks sufficient tests to modify with confidence, regardless of who wrote it and when. I used to think that legacy code was any code that was old, but this does not seem to be true. It can be freshly written code if the code does not have sufficient tests to modify with confidence.” [One of Fox and Patterson’s primary points.]

“Legacy code is an inherited piece of code which is not flexible and cannot be tested. It is code for which its logic of some part needs to be broken in order to add new features or make changes to it. So, a code is legacy code when a feature cannot be added without breaking another logic. It is also hard to rewrite legacy code due to its dependencies.” [I would not say that the second sentence (and similar points) needs to hold. Because legacy code is badly written, the odds of breaking things go up. But that’s not a precondition for it being legacy code.]

“There were a lot of definitions, but my favorite definition was that legacy code is “code that lacks sufficient test to modify with confidence. Regardless of who wrote it and when.” I think this fits well, because I was looking at some code I wrote one week ago for a side project and it didn’t make any sense at all.” [I appreciate the honesty. I would have liked to have seen the quotation cited.]

“Legacy code is preexisting code that may be outdated but is still in use because it meets a customers need. Legacy code may refer to unsupported operating systems, hardware, and formats. Legacy code can often be converted to a modern and updated version. We may also consider all code legacy, regardless of date of creation, if it can be rewritten with better programming principles.” [I would debate the accuracy of the second sentence; evidence suggests that not all code can be naturally rewritten.]

“When an application system source code type is no longer supported, it is referred to as legacy code. This also entails unsupported operating systems, hardware and formats. This code is usually converted to a modern software language and platform. Also, sometimes to maintain the same user functionality, it is simply carried onto a new environment.” [That’s also an optimistic view.]

The book gives the definition of legacy code as source code was built for an operating system that is not manufactured or used. Another definition is that it is code from an external source that can be better written in a more modern and efficient manner. [Needs citations. Incomplete.]

“According to the definition, legacy code is a source code that relates to a no-longer supported or manufactured operating system or other computer technology. It can be also referred as an inherited code from external or internal source or code that can be rewritten in better programming languages and techniques.” [“The definition” is vague. Citations?]

“Legacy code is defined multiple times throughout the chapter. The term carries a negative connotation, and is used to describe code which is outdated and isn’t maintainable, but continues to be stay in use because it still does what the system requires of it (e.g. by meeting a user need). Feathers’s definition, which is what is used in the book, is that legacy code is “code that lacks sufficient tests to modify it with confidence, regardless of who wrote it and when”.” [Really nice.]

Code smells

Think -> Share -> Pair

What code smells do you see in this example?

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

• Undescriptive variable names. y is probably “year”, but what is d?
• Magic numbers: 1980, 365, …
• Duplicated code (d -= X; y += 1). Needs to be DRYer.
• Potentially: Method naming iso8601_ordinal
• Complex Boolean expression.
  • Suggested replacement: Use a procedure/predicate.
  • Suggested replacement: Clean up the code, which seems repetitious.
• Complex nested conditionals.
• Imperative code in an object-oriented language.
• Use of object-oriented design for what is really an imperative method
• Undocumented! (Code is not self documenting.)

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