Grading Issues Fall 2007, David Matuszek

## Design

This semester, for the first time, I've been getting a lot of complaints from students who are losing points because their methods do not work. I will not be flexible about this--if your methods don't work, regardless of what the program as a whole does, your methods are wrong, and you will lose points. Usually this happens as a result of exceptionally poor program design. I'll try to explain what I mean by that.

Here is an example method description from the Dates assignment:

```    /**
* Determines what day of the week a given date falls on.
*
* @param year The year.
* @param month The month (January = 1, December = 12).
* @param day The day of the month.
* @return One of the Strings "Sunday", "Monday", "Tuesday",
*      "Wednesday", "Thursday", "Friday", or "Saturday".
*/
public String findDayOfWeek(int year, int month, int day) {
return null; // TODO replace this with the name of the weekday
}```

This method is supposed to compute which day of the week a given date falls on. All the information it needs is in the parameter list. The exact values it is supposed to return are spelled out. I should be able to call this with a year, a month, and a day, and get back exactly one of the seven specified strings. If I can't, the method is wrong.

I realize that my "pomegranite and grapes" analogy may not mean a lot to you yet, but it's the best analogy I have. If the above method works, it's like a grape--you can pull it out of its context and use it. If it only works in a given context, it's like a pomegranite seed, impossible to extract from its matrix.

For example, suppose you compute whether the year is a leap year, and store that fact in some instance variable, before you call the method. Then suppose you write the method to look at that variable in order to get its result. Now the method only works if you compute something, and store it in a special place, before you call this method. You have crippled this method--you've made it difficult to test, and impossible to use in any other circumstances. The method only sort of does what it claims to do. In short, the method is wrong.

## Using other methods

Here's a related method:

```    /**
* Asks the user for a specific date, determines what day of
* the week it falls on, and prints that day.
*/
public void findAndPrintDayOfWeek() {
// Ask the user for a date (year, month, day)
// Call findDayOfWeek to find what day of the week it is
// Print the result
}```

Because I don't expect you to already know how to design programs, I told you explicitly what to do. This method is to ask the user for some information, then call `findDayOfWeek` to get the answer, then print the answer. If this method computes the answer itself, and you don't bother getting `findDayOfWeek` correct, your program may produce the correct answers--though it's really hard to check that without typing in a dozen or so dates and checking the printed results--but your `findDayOfWeek` method is still wrong.

The DRY principle is: Don't Repeat Yourself. If you have a `findDayOfWeek` method, you should use it when you write `findAndPrintDayOfWeek`--you shouldn't repeat the computation.

This means you should not have the same code, or almost the same code, in multiple places. That makes the program longer, which makes it harder to read and maintain. It means that when you correct or update the code, you have to hunt for other places to make the same changes.

You may have noticed that this contradicts the idea that methods should be independent of context. That's correct. A method that calls other methods is dependent on those methods, and we should avoid unnecessary dependencies. In this situation, however, the DRY principle is more important. Why? Because if you call a method that is (otherwise) independent of context, and it calls other methods that are likewise independent of context, the method you are calling still works.

Using the same example, if a method `findAndPrintDayOfWeek` uses a method `findDayOfWeek`, and `findDayOfWeek` is correct, then `findAndPrintDayOfWeek` is correct. If, however, `findDayOfWeek` only works if you first call (say) `testIfLeapYear`, and otherwise might give the wrong answer, then you have a problem. The difference is that, in the first case, you can just call `findAndPrintDayOfWeek`, and you will get the correct answer. In the second case, you have to know that you need to call `testIfLeapYear` before you call `findAndPrintDayOfWeek` (and, who knows, maybe you have to call some other routine before you call `testIfLeapYear`...).

If you have to do a lot of preliminary work before you call a method, that method becomes much harder to use, and much harder to debug. If all that preliminary work is really necessary, it at least needs to be spelled out in great detail in the comments.

## Instance variables

It's great if every method can get all the information it needs from its parameter list. However, this is usually not the case.

Remember that I've said that the instance variables in a class define the state of an object. Instance variables should not be used for other purposes, such as "secret" parameter passing.

In the `TicTacToe` program, the `TicTacToeBoard` is an object that keeps track of the current state of the game. It uses an instance variable (an array) to keep track of who has played where. That variable is available to every instance method in the class. This is a correct use of an instance variable.

In the TicTacToe assignment I asked you to write the following instance method:

`boolean computerHasWon()`
Tests if the computer has won the game.

All the information necessary to determine if the computer has won is available in the state of the `TicTacToeBoard`, that is, the array. There isn't anything more to the state. So, if the board is in a given state, you should be able to call this method and find out if the computer has won.

If, instead of doing this, you check after each play whether someone has won, and save this in a variable, then have the method return the variable--then you can't test this method, or otherwise use it, without having someone play the game. This means, for example, that you can't look at

and tell whether the computer won, without playing the game to get to this point.

## The contract

Programming is a very creative activity. If you are writing a program for your own use, you can be as creative as you want in just about everything. If, however, you are working in a group on a large project, you can't just decide on a whim to add a few parameters to a method, or change its name, or make it do something different on Tuesdays. It may be creative to make an automobile with the gas pedal and the brake pedal reversed, but I wouldn't want to drive it.

A method signature--its name and its parameter list--is part of a contract between you and the other people in your group. (The rest of the contract is a documented description of what it does, and what it depends on in order to do its job.) Once a contract is agreed upon (or, in this case, specified by your instructor), you can't unilaterally change it without causing a lot of problems.

Even with only two people, this matters. Suppose your partner is supposed to write a method `gameIsOver()`, with no parameters. You write your part of the code, which uses that method. When you get your partner's code, though, your program won't compile, because his method `gameIsOver` takes two `boolean` parameters. Do you think your partner has kept his part of the bargain? Even if the method works perfectly?

Suppose there are 50 people are on a project. Suppose three or four of them occasionally decide, without consulting everyone else, to change the signature of a method that is in use by other people. What do you think the consequences will be?

In the Dates assignment I said: Use exactly the class name, method names, and method parameters as given. I even put it in boldface, just like here. That was part of the specification of the assignment. That was only your second assignment, so I thought I needed to be explicit about it. Even so, in many cases we had to modify method signatures in order to be able to test the program.

## Testing

We've gotten complaints from students who lost points for methods that worked "in most cases." Would you be happy with a calculator that gave the correct answer "in most cases"? Or a car with a steering wheel that worked "in most cases"?

Here's why this is important. A "real" program typically consists of, not just a dozen or so methods, but hundreds, or even thousands, of methods. Suppose these methods, individually, work "most of the time." Is this a good program?

To go back to the car analogy, an automobile consists of a few thousand parts. If 1% of those parts are unreliable, the car is junk; it won't run at all.

You may have heard that the majority of large programming projects fail. This is a true statement. One of the reasons for the high failure rate is inadequate testing, resulting in methods that appear to work, and in fact do work, "most of the time."

The Dates assignment had two basic tasks: computing the day of the week, and computing the interval between two dates. Instead of just testing whether each of these tasks was done correctly, we used both easy tests (interval between one day and the next) and difficult tasks (interval across year breaks, with and without leap years). Any test that passed was worth 10%. Since there were only eight tests, the other 20% was "free," provided we didn't have to make major changes in your program.