Atomic references are unnecessary when using synchronized

This code came from the book Java Concurrency Guidelines by Fred Long. I understand that a group of atomic operations is not an atomic operation. So the code below is non-compliant. To find the code, please take a look at page 23.

public class Adder {

    private AtomicReference<BigInteger> first;
    private AtomicReference<BigInteger> second;

    public Foo(BigInteger f, BigInteger s) {
        first = new AtomicReference<BigInteger>(f);
        second = new AtomicReference<BigInteger>(s);
    }

    public void update(BigInteger f, BigInteger s) {
        first.set(f);
        second.set(s);
    }

    public BigInteger add() {
        return first.get().add(second.get());
    }
}

And the right solution looks like this:

final class Adder {
    // ...
    public synchronized void update(BigInteger f, BigInteger s){
        first.set(f);
        second.set(s);
    }

    public synchronized BigInteger add() {
        return first.get().add(second.get());
    }
}

But I think the atomic references in the correct solution are redundant because synchronized guarantees both visibility and atomicity.

So my solution would look like this:

public class Addrer {

    private BigInteger first;
    private BigInteger second;

    public Addrer(BigInteger f, BigInteger s) {
        first = f;
        second = s;
    }

    public synchronized void update(BigInteger f, BigInteger s) {
        first = f;
        second = s;
    }

    public synchronized BigInteger add() {
        return first.add(second);
    }
}

Am I right?

You need to make your first and second fields private and expose those values as synchronized methods. Otherwise reading the fields directly might result in outdated or partially outdated data from BigInteger objects (non-volatile field reads are not thread safe). Then your class will be thread safe.

You might try to make those fields volatile but it won't guarantee atomicity of your update or add methods as one thread might update one field just in the middle of your update or add method execution in another thread.

public class Adder {
    private BigInteger first;
    private BigInteger second;

    public Adder(BigInteger f, BigInteger s) {
        first = f;
        second = s;
    }

    public synchronized BigInteger getFirst() {
        return first;
    }

    public synchronized BigInteger getSecond() {
        return second;
    }

    public synchronized void update(BigInteger f, BigInteger s) {
        first = f;
        second = s;
    }

    public synchronized BigInteger add() {
        return first.add(second);
    }
}