Czy warto wydzielić podobne funkcje korzystające ze .stream()?

Hej, czy da się z takich dwóch funkcji zrobić jedną działającą w zależności od getera (albo jego nazwy) podanego jako parametr?

private List<Long> mapIds(List<Car> cars) {
        return cars.stream()
        .map(Car::getId)
        .toList();
        }

private List<Long> mapOwnerIds(List<Car> cars) {
        return cars.stream()
        .map(Car::getOwnerId)
        .toList();
        }

Da się. Tylko dlaczego chcesz tak zrobić?

Da się, ale tak jak przedmówca nie widzę potrzeby. Poniżej pełny scratch:

class Scratch {
    public static void main(String[] args) {
        List<Car> cars = List.of(new Car(1L, 1L),
            new Car(2L, 1L),
            new Car(3L, 2L),
            new Car(4L, 2L),
            new Car(5L, 2L),
            new Car(6L, 3L),
            new Car(7L, 4L),
            new Car(8L, 5L),
            new Car(9L, 6L));

        System.out.println(mapIds(cars, Car::getId));
        System.out.println(mapIds(cars, Car::getOwnerId));
    }

    static class Car {
        Long id;
        Long ownerId;

        public Car(Long id, Long ownerId) {
            this.id = id;
            this.ownerId = ownerId;
        }

        public Long getId() {
            return id;
        }

        public Long getOwnerId() {
            return ownerId;
        }
    }


    private static List<Long> mapIds(List<Car> cars, Function<Car, Long> idGetter) {
        return cars.stream().map(idGetter).toList();
    }
    
}

pankeny napisał(a):

Da się, ale tak jak przedmówca nie widzę potrzeby. Poniżej pełny scratch:

Kopiuj

 // ...
    private static List<Long> mapIds(List<Car> cars, Function<Car, Long> idGetter) {
        return cars.stream().map(idGetter).toList();
    }
    
}

Ta funkcja nie powinna nazywać się mapIds, bo nie ma w niej nic co sugerowałoby że wyciąga id. Powinna nazywać się po prostu map(), mapToList() albo mapToInteger().

Riddle napisał(a):

Ta funkcja nie powinna nazywać się mapIds, bo nie ma w niej nic co sugerowałoby że wyciąga id. Powinna nazywać się po prostu map(), mapToList() albo mapToInteger().

• map niestety nazwa jest zajęte :( i zwykle bardziej generyczne :D
• mapToList brzmi źle bo na wyjściu jest lista, ale na wejściu też jest lista :D
• mapToInteger() ale przecież tam jest Long :D

BTW można by napisać generyczną metodę:

  public static <A, B> List<B> map(List<A> list, Function<A, B> f) {
        return list.stream().map(f).toList();
    }

A w zasadzie to najlepiej to by było jakby taka metoda nie była statyczna tylko metodą w List, wtedy można by napisac jak człowiek:

var ids = cars.map(Car::getId);

No, ale to już by trzeba używac vavra a nie standardowej biblioteki kolekcji :(

IMO nie warto tworzyć metody, jeśli kod jest na tyle zwięzły, że popatrzenie na niego mówi więcej niż potencjalna nazwa funkcji. W tym wypadku (wykluczając boilerplatowe .stream() i .toList() mamy tylko map(Car::getId), co jest prostsze do zrozumenia niżmapIds (a sama nazwa jest słaba, jakbym robił taką metodę to pewnie nazwałbym ją extractIds)

Nie warto.

Tak z boku do tych wszystkich komentujących "nie warto", albo "po co" - pomyśleliście, że OP uprościł problem do minimum, a całość może być dużo większa?

Ale jak? Op pokazał, która część jest wspólna (bo wkleił dwa przypadki) i powiedział co ma się różnić (getter). Pytanie wydaje się być bardzo konkretne

W jaki sposób wyklucza to to, że całość może być dużo większa? Bo jeśli chodzi o mnie to gdybym miał dwa streamy na dwadzieścia linijek, które się różnią tylko jednym map to bym zrobił dokładnie to samo co OP, tj. nie kopiował całego kodu tylko wklejony kod uprościł do minimum.

Rozbijanie kodu na bloki ma 2 powody:

• DRY
• możliwość nazwania tego bloku

Nazwy tutaj są do bani, więc niczego to nie daje. Żeby oddawać to co te metody robią, to musiałyby się nazywać carIds, nonUniqueCarOwners. W sumie ciężko powiedzieć jak miałyby się nazywać, bo wg. mnie ani w tej, ani w "połączonej" formie, nie powinny istnieć. Pomijając ich trywialność, podstawowym problemem jest, że stream API w Java jest przeznaczone do szybkiego, prostego i ~czytelnego wykonywania kilku operacji na strumieniu. Np.:

cars.stream()
  .map(Car::getOwnerId())
  .distinct()
  .toList()

Po wykorzystaniu tej mikro funkcji zwróci kolekcję, którą najprawdopodobniej kolejny raz trzeba będzie zmieniać na strumień i tracić zarówno na czytelności, jak i najprawdopodobniej na wydajności.

Java jest verbose i stream'y w Java są też verbose (gadatliwe?).

W Scali było by: cars.map(_.id) oraz cars.map(_.ownerId) i nie trzeba było by się bawić w żadne funkcje. Jak się użyje nieco lepszych streamów w Java np. Vavr, EclipseCollections to można zapisać w bardzo zwięzły sposób, podobnie jak w Scali.

W idiomatycznej Javie albo piszesz całą formułkę za każdym razem, albo sobie wyodrębniasz to do funkcji typu:

Cars.getIds(cars)
Cars.getOwnerIds(cars)

i dalej:

class Cars {
  private Cars() { }

  public static List<CarId> getIds(Collection<Cars> cars) { 
    return cars.stream().map(Car::getId).toList();
  }
}

Albo można podejść do sprawy bardziej obiektowo i zrobić sobie obiekt "kolekcja samochodów":

Cars cars = Cars.of(listOfCars);

cars.ids();
cars.ownerIds();

class Cars {
  private final Collection<Car> cars;
  public Cars(Collection<Car> cars) { this.cars = requireNonNull(cars); }

  public List<CarId> ids() { return cars.stream()... }
}

Tyle że do tego ostatniego podejścia jest przyzwyczajona coraz to mniejsza liczba osób.