Effective Java (2nd Edition)

You might be look at Joshua Bloch's book "Effective Java": http://www.amazon.com/Effective-Java-2nd-Joshua-Bloch/dp/0321356683

This is great book about Java program design.

I'd say these answers miss a trick.

Bloch, in his essential, wonderful, concise Effective Java, says, in item 47, title "Know and use the libraries", "To summarize, don't reinvent the wheel". And he gives several very clear reasons why not.

There are a few answers here which suggest methods from CollectionUtils in the Apache Commons Collections library but none has spotted the most beautiful, elegant way of answering this question:

Collection<Object> culprits = CollectionUtils.disjunction( list1, list2 );
if( ! culprits.isEmpty() ){
  // ... then in most cases you will ardently wish to do something to these culprits: 
  // at the very least, name-and-shame them.

}

Culprits: i.e. the elements which are not common to both Lists. Determining which culprits belong to list1 and which to list2 is relatively straightforward using CollectionUtils.intersection( list1, culprits ) and CollectionUtils.intersection( list2, culprits ).
However it tends to fall apart in cases like { "a", "a", "b" } disjunction with { "a", "b", "b" } ... except this is not a failing of the software, but inherent to the nature of the subtleties/ambiguities of the desired task.

NB I was at first disappointed that none of the CollectionUtils methods provides an overloaded version enabling you to impose your own Comparator (so you can redefine equals to suit your purposes).

But from collections4 4.0 there is a new class, Equator which "determines equality between objects of type T". On examination of the source code of collections4 CollectionUtils.java they seem to be using this with some methods, but as far as I can make out this is not applicable to the methods at the top of the file, using the CardinalityHelper class... which include disjunction and intersection.

I surmise that the Apache people haven't got around to this yet because it is non-trivial: you would have to create something like an "AbstractEquatingCollection" class, which instead of using its elements' inherent equals and hashCode methods would instead have to use those of Equator for all the basic methods, such as add, contains, etc. NB in fact when you look at the source code, AbstractCollection does not implement add, nor do its abstract subclasses such as AbstractSet... you have to wait till the concrete classes such as HashSet and ArrayList before add is implemented. Quite a headache.

In the mean time watch this space, I suppose. The obvious interim solution would be to wrap all your elements in a bespoke wrapper class which uses equals and hashCode to implement the kind of equality you want... then manipulate Collections of these wrapper objects.

According to the great book "Effective Java":

Item 4: Enforce noninstantiability with a private constructor

- Attempting to enforce noninstantiability by making a class abstract does not work.

- A default constructor is generated only if a class contains no explicit constructors, so a class can be made noninstantiable by including a private constructor:

// Noninstantiable utility class
public class UtilityClass
{
    // Suppress default constructor for noninstantiability
    private UtilityClass() {
        throw new AssertionError();
    }
}

Because the explicit constructor is private, it is inaccessible outside of the class. The AssertionError isn’t strictly required, but it provides insurance in case the constructor is accidentally invoked from within the class. It guarantees that the class will never be instantiated under any circumstances. This idiom is mildly counterintuitive, as the constructor is provided expressly so that it cannot be invoked. It is therefore wise to include a comment, as shown above.

As a side effect, this idiom also prevents the class from being subclassed. All constructors must invoke a superclass constructor, explicitly or implicitly, and a subclass would have no accessible superclass constructor to invoke.

A much faster alternative to serialization is implemented in Hibernate (specifically in the 2nd level cache); I don't know the details but you can check out the source code.

You may be aware that the clone() interface is broken, thus is better avoided, unless there is a really really compelling reason to use it. From Effective Java 2nd Edition, Item 11: Override clone judiciously

Given all of the problems associated with Cloneable, it’s safe to say that other interfaces should not extend it, and that classes designed for inheritance (Item 17) should not implement it. Because of its many shortcomings, some expert programmers simply choose never to override the clone method and never to invoke it except, perhaps, to copy arrays. If you design a class for inheritance, be aware that if you choose not to provide a well-behaved protected clone method, it will be impossible for subclasses to implement Cloneable.

Update: On shallow/deep cloning

From the clone() API:

Creates and returns a copy of this object. The precise meaning of "copy" may depend on the class of the object. [...]

By convention, the object returned by this method should be independent of this object (which is being cloned). To achieve this independence, it may be necessary to modify one or more fields of the object returned by super.clone before returning it. Typically, this means copying any mutable objects that comprise the internal "deep structure" of the object being cloned and replacing the references to these objects with references to the copies. If a class contains only primitive fields or references to immutable objects, then it is usually the case that no fields in the object returned by super.clone need to be modified.

So in fact, the convention is to do a deep copy.

Still, the preferred alternative is to define a copy constructor or an independent method instead of overriding clone().

I think that they wanted to apply the rule

Consider static factory methods instead of constructors

from the book Effective Java by Joshua Bloch.

The goal of that rule is that static methods can be more descriptive as constructors, because a method has a name.

For example:

ZonedDateTime.now();

is more descriptive than

new ZonedDateTime();

is, isn't it?

You might also want to read: Joshua Bloch #Item 1: Consider static factory methods instead of constructors

This is explained in Effective Java 2nd Ed., Item 73.

Thread groups were originally envisioned as a mechanism for isolating applets for security purposes. They never really fulfilled this promise, and their security importance has waned to the extent that they aren’t even mentioned in the standard work on the Java security model [Gong03].

[...] In an ironic twist, the ThreadGroup API is weak from a thread safety standpoint. To get a list of the active threads in a thread group, you must invoke the enumerate method, which takes as a parameter an array large enough to hold all the active threads. The activeCount method returns the number of active threads in a thread group, but there is no guarantee that this count will still be accurate once an array has been allocated and passed to the enumerate method. If the thread count has increased and the array is too small, the enumerate method silently ignores any threads for which there is no room in the array.

The API that lists the subgroups of a thread group is similarly flawed. While these problems could have been fixed with the addition of new methods, they haven’t, because there is no real need: thread groups are obsolete.

Prior to release 1.5, there was one small piece of functionality that was available only with the ThreadGroup API: the ThreadGroup.uncaughtException method was the only way to gain control when a thread threw an uncaught exception. This functionality is useful, for example, to direct stack traces to an application- specific log. As of release 1.5, however, the same functionality is available with Thread’s setUncaughtExceptionHandler method.

To summarize, thread groups don’t provide much in the way of useful functionality, and much of the functionality they do provide is flawed. Thread groups are best viewed as an unsuccessful experiment, and you should simply ignore their existence. If you design a class that deals with logical groups of threads, you should probably use thread pool executors (Item 68).

I usually go with something like the implementation given in Josh Bloch's fabulous Effective Java. It's fast and creates a pretty good hash which is unlikely to cause collisions. Pick two different prime numbers, e.g. 17 and 23, and do:

public override int GetHashCode()
{
    unchecked // Overflow is fine, just wrap
    {
        int hash = 17;
        // Suitable nullity checks etc, of course :)
        hash = hash * 23 + field1.GetHashCode();
        hash = hash * 23 + field2.GetHashCode();
        hash = hash * 23 + field3.GetHashCode();
        return hash;
    }
}

As noted in comments, you may find it's better to pick a large prime to multiply by instead. Apparently 486187739 is good... and although most examples I've seen with small numbers tend to use primes, there are at least similar algorithms where non-prime numbers are often used. In the not-quite-FNV example later, for example, I've used numbers which apparently work well - but the initial value isn't a prime. (The multiplication constant is prime though. I don't know quite how important that is.)

This is better than the common practice of XORing hashcodes for two main reasons. Suppose we have a type with two int fields:

XorHash(x, x) == XorHash(y, y) == 0 for all x, y
XorHash(x, y) == XorHash(y, x) for all x, y

By the way, the earlier algorithm is the one currently used by the C# compiler for anonymous types.

This page gives quite a few options. I think for most cases the above is "good enough" and it's incredibly easy to remember and get right. The FNV alternative is similarly simple, but uses different constants and XOR instead of ADD as a combining operation. It looks something like the code below, but the normal FNV algorithm operates on individual bytes, so this would require modifying to perform one iteration per byte, instead of per 32-bit hash value. FNV is also designed for variable lengths of data, whereas the way we're using it here is always for the same number of field values. Comments on this answer suggest that the code here doesn't actually work as well (in the sample case tested) as the addition approach above.

// Note: Not quite FNV!
public override int GetHashCode()
{
    unchecked // Overflow is fine, just wrap
    {
        int hash = (int) 2166136261;
        // Suitable nullity checks etc, of course :)
        hash = (hash * 16777619) ^ field1.GetHashCode();
        hash = (hash * 16777619) ^ field2.GetHashCode();
        hash = (hash * 16777619) ^ field3.GetHashCode();
        return hash;
    }
}

Note that one thing to be aware of is that ideally you should prevent your equality-sensitive (and thus hashcode-sensitive) state from changing after adding it to a collection that depends on the hash code.

As per the documentation:

You can override GetHashCode for immutable reference types. In general, for mutable reference types, you should override GetHashCode only if:

• You can compute the hash code from fields that are not mutable; or
• You can ensure that the hash code of a mutable object does not change while the object is contained in a collection that relies on its hash code.

It is a common naming convention used for static factory methods.

Joshua Bloch mentions the following about the pattern and associated naming conventions in Effective Java (2nd Edition), Item 1: Consider static factory methods instead of constructors (emphasis added):

... (a) disadvantage of static factory methods is that they are not readily distinguishable from other static methods. They do not stand out in API documentation in the way that constructors do, so it can be difficult to figure out how to instantiate a class that provides static factory methods instead of constructors. The Javadoc tool may someday draw attention to static factory methods. In the meantime, you can reduce this disadvantage by drawing attention to static factories in class or interface comments, and by adhering to common naming conventions. Here are some common names for static factory methods:

• valueOf—Returns an instance that has, loosely speaking, the same value as its parameters. Such static factories are effectively type-conversion methods.

• of—A concise alternative to valueOf, popularized by EnumSet (Item 32).

...

So, as others have pointed out, "of" means the English word "of", and is not an abbreviation. But one of the reasons for using this convention is to make it easier to find out if a class provides a factory method because static factories don't show in a separate section in the JavaDocs (compared to constructors).

An added benefit, in my opinion, with using concisely and desciptively named static factories, is that it makes the code read like prose, which isn't the case if it's littered with new constructor calls.

On the left hand side you're declaring the type of the variable, lst. Since lst's type is List you can only access methods of a List, even if the object lst points to is really a LinkedList. There's an inherent tradeoff between declaring a variable of a concrete type like LinkedList (access to more methods / behavior) vs. a more abstract interface (safer, better compartmentalized code).

This is a big topic, and there isn't one simple answer for when to do one vs. the other (though there's lots of advice out there about it!) - you'll need to figure out which is appropriate for your use case.

Effective Java - Item 52: Refer to objects by their interfaces is a pretty canonical citation for this issue, and as the title implies suggest preferring List rather than LinkedList.

only let classes implement Serializable that have to be passed by value over the network.

Your professor is suggesting you minimize your use of Serializable to areas where it's strictly needed.

This is because serialization is a strong candidate for leaking implementation. Implementing Serializable shows intent of serialization (even if the object is never actually serialized), which imposes the idea that developers should take caution when modifying those classes to avoid breaking software.

Joshua Bloch covers this in his book Effective Java.

The moment you serialize an object, the class that it was instantiated from can no longer be modified without special treatment^†. If you modify the class, the binary representation will no longer match the objects already serialized. Thus deserialization of any objects serialized before modifying the class will fail.

If a type implements Serializable, it has the potential to be serialized. If an instance of that type was serialized, you may break code by modifying it's implementation.

Since there's no easy way of knowing for sure that an instance of a serializable type has been serialized (albeit you may not intend for objects to be serialized), developers take strong caution when modifying implementations of those types.

^† - This could be avoided by properly versioning your serializable types, but due to the potential of versioning a type that had no contract change (with no compile time error handling support to notify you), it's best to keep explicit versioning minimal to avoid adding excess complexity to your design.

In Joshua Bloch's Effective Java, Item 5: "Avoid creating unnecessary objects", he posts the following code example:

public static void main(String[] args) {
    Long sum = 0L; // uses Long, not long
    for (long i = 0; i <= Integer.MAX_VALUE; i++) {
        sum += i;
    }
    System.out.println(sum);
}

and it takes 43 seconds to run. Taking the Long into the primitive brings it down to 6.8 seconds... If that's any indication why we use primitives.

The lack of native value equality is also a concern (.equals() is fairly verbose compared to ==)

for biziclop:

class Biziclop {

    public static void main(String[] args) {
        System.out.println(new Integer(5) == new Integer(5));
        System.out.println(new Integer(500) == new Integer(500));

        System.out.println(Integer.valueOf(5) == Integer.valueOf(5));
        System.out.println(Integer.valueOf(500) == Integer.valueOf(500));
    }
}

Results in:

false
false
true
false

_^EDIT Why does (3) return true and (4) return false?

Because they are two different objects. The 256 integers closest to zero [-128; 127] are cached by the JVM, so they return the same object for those. Beyond that range, though, they aren't cached, so a new object is created. To make things more complicated, the JLS demands that at least 256 flyweights be cached. JVM implementers may add more if they desire, meaning this could run on a system where the nearest 1024 are cached and all of them return true... #awkward

It is very true.

Because setters are always public methods. And if you class is not final then there is issue of alien method call. Which is not thread safe i.e. it is known as escaping of this reference. So from a constructor if you are calling a method it should be final or private. Else safe initialization of object will not happen which causes many bugs in real systems.

Apart from the above we should never call public method from the constructor because if the class is intended for inheritance than Constructors must not invoke overridable methods, directly or indirectly.

If you violate this rule, program failure will result. The superclass constructor runs before the subclass constructor, so the overriding method in the subclass will be invoked before the subclass constructor has run. If the overriding method depends on any initialization performed by the subclass constructor, the method will not behave as expected.

source.

You're not trying to implement Joshua Bloch's typesafe hetereogeneous container pattern are you? Basically:

public class Favorites {
  private Map<Class<?>, Object> favorites =
    new HashMap<Class<?>, Object>();

  public <T> void setFavorite(Class<T> klass, T thing) {
    favorites.put(klass, thing);
  }

  public <T> T getFavorite(Class<T> klass) {
    return klass.cast(favorites.get(klass));
  }

  public static void main(String[] args) {
    Favorites f = new Favorites();
    f.setFavorite(String.class, "Java");
    f.setFavorite(Integer.class, 0xcafebabe);
    String s = f.getFavorite(String.class);
    int i = f.getFavorite(Integer.class);
  }
}

From Effective Java (2nd edition) and this presentation.

I would say both NumberFormatException and MyBusinessException are useful but in different cases.

They usually appear at different levels of class hierarchy: for example NumberFormatException is a lower-level exception and you might not want to expose it at a higher level (e.g. user interface) if the user of it has no power to recover from it. In this case it is more elegant to just throw MyBusinessException and display an informative message that explains for example that something in a previous step was badly supplied or some internal processing error occurred and he/she needs to restart the process.

On the other hand, if your function is used at an intermediate-level (e.g. API) and the developer has the means to recover from the exceptional behavior, NumberFormatException is more useful, as it can be dealt with programmatically and the flow of the application might continue with minimal interruption (e.g. supply a default valid number). Alternatively, this can indicate a flaw/bug in the code that should be fixed.

For details about how to follow best practice in using exceptions, read Item 61 - Throw exceptions appropriate to the abstraction from Effective Java by Joshua Bloch.

The book Effective Java gives two more reasons for "memory leaks":

• Once you put object reference in Cache and forget that it's there. The reference remains in cache long before becoming irrelevant. Solution is to represent cache as a WeakHashMap
• in an API where clients register callbacks and don't re-register them explicitly. Solution is to store only weak references to them.

The best approach is Long.valueOf(str) as it relies on Long.valueOf(long) which uses an internal cache making it more efficient since it will reuse if needed the cached instances of Long going from -128 to 127 included.

Returns a Long instance representing the specified long value. If a new Long instance is not required, this method should generally be used in preference to the constructor Long(long), as this method is likely to yield significantly better space and time performance by caching frequently requested values. Note that unlike the corresponding method in the Integer class, this method is not required to cache values within a particular range.

Generally speaking, it is a good practice to use the static factory method valueOf(str) of a wrapper class like Integer, Boolean, Long, ... since most of them reuse instances whenever it is possible making them potentially more efficient in term of memory footprint than the corresponding parse methods or constructors.

Excerpt from Effective Java Item 1 written by Joshua Bloch:

You can often avoid creating unnecessary objects by using static factory methods (Item 1) in preference to constructors on immutable classes that provide both. For example, the static factory method Boolean.valueOf(String) is almost always preferable to the constructor Boolean(String). The constructor creates a new object each time it’s called, while the static factory method is never required to do so and won’t in practice.

You should probably use String.hashCode().

If you really want to implement hashCode yourself:

Do not be tempted to exclude significant parts of an object from the hash code computation to improve performance -- Joshua Bloch, Effective Java

Using only the first five characters is a bad idea. Think about hierarchical names, such as URLs: they will all have the same hash code (because they all start with "http://", which means that they are stored under the same bucket in a hash map, exhibiting terrible performance.

Here's a war story paraphrased on the String hashCode from "Effective Java":

The String hash function implemented in all releases prior to 1.2 examined at most sixteen characters, evenly spaced throughout the string, starting with the first character. For large collections of hierarchical names, such as URLs, this hash function displayed terrible behavior.

It's extremely bad practice to let equals() or hashCode() throw exceptions (I'd encourage you to read Effective Java for the details).

Also, your methods are unnecessarily complex. Ever since Java 7, this is pretty much the canonical way of writing these methods:

@Override
public boolean equals(Object o) {
    if (this == o) {
        return true;
    }else if (o instanceof MyClass) {
        // some will argue that the line above should have a
        // .getClass() check instead of an instanceof check.
        // those people also need to read Effective Java :-)
        MyClass that = (MyClass) o;
        return Objects.equals(this.classA, that.classA)
            && Objects.equals(this.classB, that.classB)
    } else {
        return false;
    } 
}

@Override
public int hashCode() {
    return Objects.hash(classA, classB);
}

1. What you should be more concerned with here, is not efficiency, but scope. Generally, you should strive to keep your variables as locally scoped as possible. This means, if you only need x within the loop, you should define it within the loop.

You get a number of benefits with keeping your variables as locally scoped as possible:

• Your code will be much more readable to someone else
• You won't accidentally assign to, or use the value of a variable you defined further up in your code that is still in scope, thus minimizing errors in your program
• Most importantly, the garbage collector will free up any memory used by the variable as soon as it goes out of scope, keeping your program's performance high, and memory usage low.

You can read up more on variable scope and best practices from Josh Bloch's excellent book, "Effective Java" (scope is discussed in items 13 and 45). You might also want to read item 55, which discusses why it is important to optimize judiciously.

2. For the second part of your question, see The Skeet's answer here.

Here's an example:

public static void main(String[] args) {
    for(int i=0; i<getSize(); i++) {
        System.out.println("i: " + i);
    }
}

private static int getSize() {
    int size = new Random().nextInt(10);
    System.out.println("size: " + size);
    return size;
}

This outputs:

size: 2
i: 0
size: 4
i: 1
size: 4
i: 2
size: 8
i: 3
size: 0

Notice how getSize() is called for every iteration of the loop. In your example, calling .length won't make a huge difference, as the JIT runtime will know how to optimize this call. But imagine getSize() was a more complex operation, like counting the number of rows in a database table. Your code will be super slow as every iteration of the loop will call getSize(), resulting in a database roundtrip.

This is when you would be better off evaluating the value before hand. You can do this and still retain minimal scope for size, like this:

public static void main(String[] args) {
    for(int size = getSize(), i=0; i<size; i++) {
        System.out.println("i: " + i);
    }
}


private static int getSize() {
    int size = new Random().nextInt(10);
    System.out.println("size: " + size);
    return size;
}

size: 5
i: 0
i: 1
i: 2
i: 3
i: 4

Notice how getSize() is only called once, and also, the size variable is only available inside the loop and goes out of scope as soon as the loop completes.

In recent years, inheritance is often treated like code-smell, because it can lead to different problems: https://dzone.com/articles/is-inheritance-dead

If we talk in pure performance term, an empty array takes about 8 bytes in RAM (4 bytes store length and 4 bytes a reference, but it is a little platform-dependent: How much space array occupies). So, even if you have a thousand of such objects, one array field will take approximately 1_000 * 8 bytes ~ 8 KBytes in RAM.

As you probably know, nowadays phones usually contain > 1 GByte of RAM. However, don't forget that your app usually can take from 60 to 192 MBytes of RAM (Detect application heap size in Android).

In any case, it is more than enough not to count every little field that you are adding to your class.

However, going back to my first statement, I suggest you to think about solving the problem using composition instead of inheritance, as it is suggested in Effective Java

Update
About performance, I would suggest you to read this topic: The performance impact of using instanceof in Java Are you sure that you need such type of premature optimization? Or is it more a theoretical question than practical?