Feedback on intermediate evaluation

Below are a few items regarding the hands-on part of the evaluation.

The main lesson I would draw is that many of you apparently derived excessive confidence that if the provided tests passed, then the code was correct.

This is an important and critical mistake: the fact that you have not witnessed a bug does not mean there are none. The tests provided were fairly shallow and not sufficient to assess whether your code fulfilled the contract you were asked for (and, as advertised, the tests used for evaluation were more thorough).

Below I treat the elements that were problematic for a significant number of students and that I feel are worth going back to.

Equals

Main problem: many only implemented equals accepting a Point parameter

    public boolean equals(Point p) {  // INCORRECT
       return p.x == this.x && p.y == this.y;

The method above does NOT override any behavior because it is not the same as the method defined in Object that accepts an Object as a parameter. Among other problems, it would lead to the following behavior.

    Point a = new Point(1, 1);
    Point b = new Point(1, 1);
    Ojbect c = (Object) new Point(1, 1);
    a.equals(b); // return true
    a.equals(c); // returns false (calls the default implementation in Ojbect)

Below is a valid implementation that does override the Object.equals(Object o) method.

@Override
public boolean equals(Object o) {  // YES EQUALS SHOULD WORK FOR ANY OBJECT
    if (o == null) {
        return false;
    } else if (o instanceof Point p) {
        return p.x == this.x && p.y == this.y;
    } else {
        return false;
    }

Hash code

The only requirement for hashCode is that if two objects are equals they have the same hashCode.

Thus, there are many correct implementations. Always returning 0 is a correct implementation :

    @Override 
    public int hashCode() {
        return 0; // CORRECT BUT VERY BAD
    }

However, it is a terrible implementation: all instances of Point would have the same hashCode. If we were to place them in a hash table, they would all go in the same location, and you would lose all benefit of using a hash table.

The objective of hashing is to combine the values of the different elements into a mostly unique bit pattern.

This following hash implementation is correct and reasonable but still produces many collisions on common patterns (e.g. Point(1, 2) and Point(2, 1) have the same hash)

   return x + y;

The following one is better one, that is fast simple and produce be collisions:

   return x * 31 + y;

But the best option is to use the hasher in the java standard library:

   return Objects.hash(x, y);

There were some points for having a correct hash and some others for a hash with few collisions.

I also saw a number of incorrect hash implementations, for instance:

   return x / y; // INCORRECT: would crash if y=0

Above the problem is that the implementer did not consider all edge cases in the implementation, leading to a division by zero for some inputs.

Counting characters

On the counting characters part, there were less problematic patterns coming up. But the two I want to call out for are:

Broken parallelism

I saw many cases of essentially this pattern :

   for (int i = 0 ; i< 10 ; i++) {
     Thread t = new Thread(() -> doWork());
     t.start()
     t.join()
   }

In this example, 10 new threads are started but NOTHING is done in parallel : the join ensures the started thread is finished before starting the next one.

The correct way to do this is the following (starts all the threads and then wait for all of them to finish).

    List<Thread> threads = new ArrayList();  
    for (int i = 0 ; i< 10 ; i++) {
      Thread t = new Thread(() -> doWork());
      t.start()
      threads.push(t);
    }
    for (Thread t : threads) {
      t.join()
    }

Breaking backward compatibility

(This was very uncommon but important enough to call out for.)

An important problem was the modification of the methods signature, that would break compatibility with other code.

For example changing the method definition of the first method to the second may break ANY code that uses it (notably all the tests):

    public static int countOccurrencesInThread(String str, char character) { 
    public static int countOccurrencesInThread(String str, char character) throws InterruptedException {

This would lead to a compilation error for any code that relied on the previous signature.