GlenKPeterson / Paguro

Generic, Null-safe, Immutable Collections and Functional Transformations for the JVM
Other
312 stars 24 forks source link

MutVector<F>#replace() problem #46

Closed axkr closed 2 years ago

axkr commented 2 years ago

MutVector<F>#replace()does not replace the value at index 1 in this case:

Also the question is the replace() method the right way to replace a value in a vector?

GlenKPeterson commented 2 years ago

This unit test passes:

    @Test public void testMutable() {
        List<Integer> control = new ArrayList<>();
        MutList<Integer> test = PersistentVector.emptyMutable();
        final int SEVERAL = 2000; // more than 1024 so 3 levels deep.
        for (int i = 0; i < SEVERAL; i++) {
            control.add(i);
            test.append(i);
            assertEquals(control.size(), test.size());
        }

        for (int i = 0; i < SEVERAL; i++) {
            assertEquals(control.get(i), test.get(i));
        }

        for (int i = 0; i < SEVERAL; i++) {
            control.set(i, i + 10);
            test.replace(i, i + 10);
            assertEquals(control.size(), test.size());
            assertEquals(control, test);
        }

Can you alter it to make it fail? If you can do that, you don't need to answer any of my other questions. Otherwise:

If I understand what I'm looking at in your image, you have a mutable vector with a list as the first element and Doubles or Floats as the other visible elements. You just called replace with object id=314 and idx=1 and it looks like it put object id=314 (-92.4031) at index 1 and is ready to return the correctly mutated-in-place vector.

Questions about your example:

Questions about reproducing this issue:

axkr commented 2 years ago

Seems to be a problem with the MutVector#replace() and call of MutVector#editableArrayFor() method.

Example for 3.6.0 version from Maven Central:

import org.organicdesign.fp.StaticImports;
import org.organicdesign.fp.collections.ImList;
import org.organicdesign.fp.collections.MutList;

public class PersistentVectorTest {
  public static void main(String[] args) {
    ImList<String> v = StaticImports.vec("0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "10",
        "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", "23", "24", "25",
        "26", "27", "28", "29", "30", "31", "32", "33", "34", "35", "36", "37", "38", "39", "40",
        "41", "42", "43", "44", "45", "46", "47", "48", "49", "50", "51", "52", "53", "54", "55",
        "56", "57", "58", "59", "60", "61", "62", "63", "64", "65", "66", "67", "68", "69", "70",
        "71", "72", "73", "74", "75", "76", "77", "78", "79", "80", "81", "82", "83", "84", "85",
        "86", "87", "88", "89", "90", "91", "92", "93", "94", "95", "96", "97", "98", "99");
    MutList<String> m = v.mutable();
    m.replace(0, "bug");
    System.out.println(m.get(0).toString());
  }
}
GlenKPeterson commented 2 years ago

Thanks for finding this bug! I will fix, but maybe not until next weekend.

GlenKPeterson commented 2 years ago

Thank you so much! This should be fixed in 3.7.0 which I just released. If you get a chance to test it, please leave a comment here and I'll close the issue.

axkr commented 2 years ago

Seems to be fixed