New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clear* functionality for @Singular fields. #967

Closed
jmax01 opened this Issue Nov 16, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@jmax01

jmax01 commented Nov 16, 2015

Currently there is no clean way to clear @Singular fields.

This is especially problematic when using toBuilder() as @Singular fields retain their original contents.

The tests below illustrate the problem and show what is required to workaround it.

Ideally clear* methods should be generated for each @Singular field.

package lombok.javac.handlers;

import static org.junit.Assert.*;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import lombok.Builder;
import lombok.Singular;
import lombok.Value;

import org.junit.Test;

/**
 * The Class WithBuilderTest.
 */
public class ClearSingular {

    /**
     * Collections are not annotated with {@link Singular}. {@link Builder#toBuilder()} constructed instances can be
     * cleared by setting with empty collection.
     */
    @Value
    @Builder(toBuilder = true)
    public static class MyClassWithNonSingular {

        /** The strings. */
        List<String> strings;

        /** The numbers by strings. */
        Map<String, Number> numbersByStrings;

    }

    /**
     * Collections are annotated with {@link Singular}. {@link Builder#toBuilder()} constructed instances CANNOT be
     * cleared by setting with empty collection.
     */
    @Value
    @Builder(toBuilder = true)
    public static class MyClassWithSingular {

        /** The strings. */
        @Singular
        List<String> strings;

        @Singular
        /** The numbers by strings. */
        Map<String, Number> numbersByStrings;
    }

    /**
     * Collections are annotated with {@link Singular}, but a very unpleasant workaround allows {@link Singular}s to be
     * cleared. {@link Builder#toBuilder()} constructed instances can be cleared by calling clear* methods.
     * Workaround is dangerous as it exposes Lombok's {@link Singular} internals.
     */
    @Value
    @Builder(toBuilder = true)
    public static class MyClassWithSingularWithWorkaround {

        /** The strings. */
        @Singular
        List<String> strings;

        @Singular
        Map<String, Number> numbersByStrings;

        /**
         * With awful workaround.

         */
        public static class MyClassWithSingularWithWorkaroundBuilder {

            /**
             * Awful workaround.
             * @return builder
             */
            public MyClassWithSingularWithWorkaroundBuilder clearNumbersByStrings() {
                this.numbersByStrings$key = null;
                this.numbersByStrings$value = null;
                return this;
            }

            /**
             * Awful workaround.
             * @return builder
             */
            public MyClassWithSingularWithWorkaroundBuilder clearStrings() {
                this.strings = null;
                return this;
            }
        }
    }

    /**
     * Can clear non-singular.
     */
    @Test
    public void testClearingNonSingular() {

        Map<String, Number> numbersByStrings = new HashMap<>();
        numbersByStrings.put("1", 1);

        MyClassWithNonSingular populated = MyClassWithNonSingular.builder()
            .strings(Arrays.asList("FIRST"))
            .numbersByStrings(numbersByStrings)
            .build();

        MyClassWithNonSingular cleared = populated.toBuilder()
            .strings(new ArrayList<>())
            .numbersByStrings(new HashMap<>())
            .build();

        int clearedMapSize = cleared.getNumbersByStrings()
            .size();

        int clearedListSize = cleared.getStrings()
            .size();

        // Passes!
        assertTrue("Both should be empty but clearedMapSize: " + clearedMapSize + " clearedListSize: "
                + clearedListSize, (clearedMapSize == 0 && clearedListSize == 0));
    }

    /**
     * No way to clear singular!.
     */
    @Test
    public void testClearingSingular() {

        MyClassWithSingular populated = MyClassWithSingular.builder()
            .string("FIRST")
            .numbersByString("1", 1)
            .build();

        MyClassWithSingular cleared = populated.toBuilder()
            .strings(new ArrayList<>())
            .numbersByStrings(new HashMap<>())
            .build();

        int clearedMapSize = cleared.getNumbersByStrings()
            .size();

        int clearedListSize = cleared.getStrings()
            .size();

        // Fails!
        assertTrue("Both should be empty but clearedMapSize: " + clearedMapSize + " clearedListSize: "
                + clearedListSize, (clearedMapSize == 0 && clearedListSize == 0));

    }

    /**
     * Test workaround.
     */
    @Test
    public void testClearingSingularWorkaround() {

        MyClassWithSingularWithWorkaround populated = MyClassWithSingularWithWorkaround.builder()
            .string("FIRST")
            .numbersByString("1", 1)
            .build();

        MyClassWithSingularWithWorkaround cleared = populated.toBuilder()
            .clearStrings()
            .clearNumbersByStrings()
            .build();

        int clearedMapSize = cleared.getNumbersByStrings()
            .size();

        int clearedListSize = cleared.getStrings()
            .size();

        // Passes!
        assertTrue("Both should be empty but clearedMapSize: " + clearedMapSize + " clearedListSize: "
                + clearedListSize, (clearedMapSize == 0 && clearedListSize == 0));

    }
}
@rzwitserloot

This comment has been minimized.

Show comment
Hide comment
@rzwitserloot

rzwitserloot Nov 16, 2015

Owner

Working on it now as part of a big builder update.

Owner

rzwitserloot commented Nov 16, 2015

Working on it now as part of a big builder update.

@rzwitserloot

This comment has been minimized.

Show comment
Hide comment
@rzwitserloot

rzwitserloot Nov 16, 2015

Owner

Done; will be in v1.16.8. Pushed to git on branch 'builderUpdate'.

Owner

rzwitserloot commented Nov 16, 2015

Done; will be in v1.16.8. Pushed to git on branch 'builderUpdate'.

@Controlix

This comment has been minimized.

Show comment
Hide comment
@Controlix

Controlix Apr 27, 2016

Any news on this issue? It is closed, but I do not find this in the v1.16.8 version as stated above.

Controlix commented Apr 27, 2016

Any news on this issue? It is closed, but I do not find this in the v1.16.8 version as stated above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment