How to change this merge() method for a Multi-Value Register CRDT implementation?

Question

I have the following problem: In this simple implementation of a MVR-CRDT the merge() method should add all values that are not null to the merged replica. For that to happen i used a for loop which iterates through both ArrayLists. At every index where a value is present it compares the vector clocks and overwrites the value of the highest clock at said index. It seems to not be working as intended. I would be glad if somebody can point out the problem.

import java.util.ArrayList;
import java.util.Collections;

public class MVR_CRDT{

    //variables
    static int size;
    private ArrayList<Integer> vectorClock;
    private ArrayList<Object> values;
    
    //constructor
    public MVR_CRDT(int size) {
        this.vectorClock = new ArrayList<Integer>(Collections.nCopies(size, 0));
        this.values = new ArrayList<Object>(Collections.nCopies(size, null));
    }    
    
    //sets a value and increments its corresponding vector clock
    public void setValue(int index, Object value) {
        //check if value is different
        if (value != this.values.get(index)){
            this.vectorClock.set(index, this.vectorClock.get(index) + 1);
            this.values.set(index, value);
            //goes over the other indexes and empties their value
            for (int i = 0; i < size; i++) {
                while (i != index){
                    this.values.set(i, null);
                }
            }
        }
    }

    //the second replica merges into the first
    public static void merge(MVR_CRDT first, MVR_CRDT second){
            //loops through the ArrayList
            for (int i = 0; i < size; i++) {
                //if there is a value to be merged
                if (second.values.get(i) != null){
                    first.vectorClock.set(i, max(first.vectorClock.get(i), second.vectorClock.get(i)));
                    first.values.set(i, second.values.get(i));
                }
            }
    }
    
    private static Integer max(Integer integer, Integer integer2) {
        return null;
    }

    //gets the value at index position of a replica
    public Object getValue(int index) {
        return this.values.get(index);
    }

    public Integer getVector(int index) {
        return this.vectorClock.get(index);
    }

    //prints the replica
    public void query() {
        System.out.println("Vector Clock: " + this.vectorClock.toString());
        System.out.println("Values: " + this.values.toString() + "\n");
    }

    public static void main(String[] args) {
        MVR_CRDT Alice = new MVR_CRDT(3);
        MVR_CRDT Bob = new MVR_CRDT(3);
        MVR_CRDT Charlie = new MVR_CRDT(3);

        Alice.setValue(0, "Hello");
        Alice.setValue(0, "Hello"); //to show that Alices counter does not increase
        System.out.println("Alice's replica: ");
        Alice.query();

        Bob.setValue(1, 12312);
        System.out.println("Bob's replica: ");
        Bob.query();

        // Charlie.setValue(2, false);

        merge(Alice, Bob);
        System.out.println("Alice's replica after merging: ");
        Alice.query();

        // System.out.println(Bob.getValue(1) + " , " + Bob.getVector(1));

    }

}

Output after merge expected:

Vector Clock: [1, 1, 0] Values: [Hello, 12312, null]

Main method output:

Alice's replica: 
Vector Clock: [1, 0, 0]
Values: [Hello, null, null]

Bob's replica:
Vector Clock: [0, 1, 0]
Values: [null, 12312, null]

Alice's replica after merging:
Vector Clock: [1, 0, 0]
Values: [Hello, null, null]

Answer 1

Looking at your code, several improvements can be done. I start with the one you asked for, so you can stop after that:

Your method merge uses the static variable size as stop criterion in the for loop, that is never explicitly initialized, so it is implicitly initialized to 0 - the loop in merge never does a single pass, so your output shows an unmodified Alice.

But even if it would use a "better" size, like e.g. first.vectorClock.size() it would not output the correct result - but a different one:

Alice's replica after merging: 
Vector Clock: [1, null, 0]
Values: [Hello, 12312, null]

This is because your max method always returns null. I would recommend that you use a built in max method like Integer.max(int, int). Then your expected result will be output.

---

And now for some other findings (you could stop reading, your original problem is solved): Please use an established naming convention when you post code here, for Java this is in the Java Language Specification, chapter 6. Names, section 6.1. Declarations (it starts after the long nested list there, written in italic).

So for your class name that should be MvrCrdt or maybe better MultiValueRegisterCRDT - or even longer? Your instances Alice and Bob should start with a lowercase letter and be alice and bob.

Your code leftovers and commented code should be removed, before posting here.

Declarations: your member fields do not need to know the implementation of the list, it's sufficient to use List<Integer> and List<Object>, use ArrayList<> in the constructor only with the new operator!

As you never reassign the member fields vectorClock and values, you should make them final.

Constructor: You don't need to specify the type that the list stores on the assignment, it's sufficient to have this.vectorClock = new ArrayList<>(Collections.nCopies(size, 0));

Method setValue: You have the same issue with size in your setValue method, replace it with this.values.size().

In the while loop inside your for loop the values of i and index never change inside the while loop, so you should replace the while with an if or you change the loop-body. Otherwise you will have an endless loop, if the loop is ever entered! For now you didn't face this problem, since the outer for loop never was entered.

Method merge: That method should neither be static nor take two parameters. It always modifies the first input, so it could as well just take the object to merge into the current instance and manipulate this instead of first:

public void merge(MultiValueRegisterCrdt other) {
  //loops through the ArrayList
  for (int i = 0; i < this.vectorClock.size(); i++) {
    //if there is a value to be merged
    if (other.values.get(i) != null) {
      this.vectorClock.set(i, Integer.max(this.vectorClock.get(i), other.vectorClock.get(i)));
     this.values.set(i, other.values.get(i));
    }
  }
}

With that modification you would have to use it so: alice.merge(bob);

Method query: you could replace this with an override of toString like:

@Override
public String toString() {
  return "MultiValueRegisterCrdt{" +
      "vectorClock=" + vectorClock +
      ", values=" + values +
      '}';
}

Then you can use it like System.out.println("Alice's replica: \n" + alice);