spring-projects / spring-data-redis

Provides support to increase developer productivity in Java when using Redis, a key-value store. Uses familiar Spring concepts such as a template classes for core API usage and lightweight repository style data access.
https://spring.io/projects/spring-data-redis/
Apache License 2.0
1.77k stars 1.17k forks source link

Elimate Code Duplication in DefaultRedisList #2996

Closed kjb512 closed 2 months ago

kjb512 commented 2 months ago

There is code duplication in the DefaultRedisList methods. Specifically, the usage of listOps.rightPush/leftPush followed by a call to cap() is repeated in multiple places. This can be refactored to enhance code maintainability and readability.


    @Override
    public boolean add(E value) {
        listOps.rightPush(value);
        cap();
        return true;
    }
    @Override
    public boolean offer(E element) {
        listOps.rightPush(element);
        cap();
        return true;
    }
    @Override
    public void addFirst(E element) {
        listOps.leftPush(element);
        cap();
    }
    @Override
    public void add(int index, E element) {

        if (index == 0) {
            listOps.leftPush(element);
            cap();
            return;
        }

        int size = size();

        if (index == size()) {
            listOps.rightPush(element);
            cap();
            return;
        }

        if (index < 0 || index > size) {
            throw new IndexOutOfBoundsException();
        }

        throw new IllegalArgumentException("Redis supports insertion only at the beginning or the end of the list");
    }


Proposed Solution:

To enhance consistency and readability, I propose refactoring the method names to addFirst and addLast. This will provide a clear indication of where the element is being added, compared to the more generic add.

AS-IS

    @Override
    public void add(int index, E element) {

        if (index == 0) {
            listOps.leftPush(element);
            cap();
            return;
        }

        int size = size();

        if (index == size()) {
            listOps.rightPush(element);
            cap();
            return;
        }

        if (index < 0 || index > size) {
            throw new IndexOutOfBoundsException();
        }

        throw new IllegalArgumentException("Redis supports insertion only at the beginning or the end of the list");
    }

TO-BE

    @Override
    public void add(int index, E element) {

        if (index == 0) {
            addFirst(element);
            return;
        }

        int size = size();

        if (index == size()) {
            addLast(element);
            return;
        }

        if (index < 0 || index > size) {
            throw new IndexOutOfBoundsException();
        }

        throw new IllegalArgumentException("Redis supports insertion only at the beginning or the end of the list");
    }