Po pierwsze ogólny przypadek: użycie flagi do sprawdzenia, czy jakiś element kolekcji spełnia określony warunek, nie jest rzadkością. Ale wzór, który najczęściej widziałem w celu rozwiązania tego problemu, to przesunięcie czeku w dodatkową metodę i bezpośredni powrót z niego (jak opisał Kilian Foth w swojej odpowiedzi ):
private <T> boolean checkCollection(Collection<T> collection)
{
for (T element : collection)
if (checkElement(element))
return true;
return false;
}
Od wersji Java 8 istnieje bardziej zwięzły sposób, używając Stream.anyMatch(…):
collection.stream().anyMatch(this::checkElement);
W twoim przypadku prawdopodobnie wyglądałoby to tak (zakładając list == entry.getValue()w twoim pytaniu):
map.values().stream().anyMatch(list -> list.size() > limit);
Problemem w twoim konkretnym przykładzie jest dodatkowe połączenie z fillUpList(). Odpowiedź zależy w dużej mierze od tego, co ma zrobić ta metoda.
Uwaga dodatkowa: wezwanie do fillUpList()nie ma większego sensu, ponieważ nie zależy od elementu, który aktualnie iterujesz. Wydaje mi się, że jest to konsekwencja zmniejszenia kodu w celu dopasowania go do formatu pytania. Ale właśnie to prowadzi do sztucznego przykładu, który jest trudny do interpretacji, a zatem trudny do uzasadnienia. Dlatego tak ważne jest podanie minimalnego, kompletnego i weryfikowalnego przykładu .
Zakładam więc, że rzeczywisty kod przekazuje prąd entrydo metody.
Ale jest więcej pytań, które należy zadać:
- Czy listy na mapie są puste przed dotarciem do tego kodu? Jeśli tak, to dlaczego istnieje już mapa, a nie tylko lista lub zestaw
BigIntegerkluczy? Jeśli nie są puste, dlaczego musisz wypełnić listy? Jeśli na liście znajdują się już elementy, czy nie jest to aktualizacja lub inne obliczenie w tym przypadku?
- Co powoduje, że lista staje się większa niż limit? Czy jest to warunek błędu, czy często się zdarza? Czy jest to spowodowane nieprawidłowymi danymi wejściowymi?
- Czy potrzebujesz list obliczonych do momentu, gdy osiągniesz listę większą niż limit?
- Co robi część „ Zrób coś ”?
- Czy ponownie zaczynasz napełnianie po tej części?
To tylko niektóre pytania, które przyszły mi do głowy, gdy próbowałem zrozumieć fragment kodu. Tak więc, moim zdaniem, jest to prawdziwy zapach kodu : Twój kod nie komunikuje jasno intencji.
Może to oznaczać albo („wszystko albo nic”, a osiągnięcie limitu oznacza błąd):
/**
* Computes the list of all foo strings for each passed number.
*
* @param numbers the numbers to process. Must not be {@code null}.
* @return all foo strings for each passed number. Never {@code null}.
* @throws InvalidArgumentException if any number produces a list that is too long.
*/
public Map<BigInteger, List<String>> computeFoos(Set<BigInteger> numbers)
throws InvalidArgumentException
{
if (numbers.isEmpty())
{
// Do you actually need to log this here?
// The caller might know better what to do in this case...
logger.info("Nothing to compute");
}
return numbers.stream().collect(Collectors.toMap(
number -> number,
number -> computeListForNumber(number)));
}
private List<String> computeListForNumber(BigInteger number)
throws InvalidArgumentException
{
// compute the list and throw an exception if the limit is exceeded.
}
Lub może to oznaczać („aktualizacja do pierwszego problemu”):
/**
* Refreshes all foo lists after they have become invalid because of bar.
*
* @param map the numbers with all their current values.
* The values in this map will be modified.
* Must not be {@code null}.
* @throws InvalidArgumentException if any new foo list would become too long.
* Some other lists may have already been updated.
*/
public void updateFoos(Map<BigInteger, List<String>> map)
throws InvalidArgumentException
{
map.replaceAll(this::computeUpdatedList);
}
private List<String> computeUpdatedList(
BigInteger number, List<String> currentValues)
throws InvalidArgumentException
{
// compute the new list and throw an exception if the limit is exceeded.
}
Lub to („zaktualizuj wszystkie listy, ale zachowaj oryginalną listę, jeśli stanie się zbyt duża”):
/**
* Refreshes all foo lists after they have become invalid because of bar.
* Lists that would become too large will not be updated.
*
* @param map the numbers with all their current values.
* The values in this map will be modified.
* Must not be {@code null}.
* @return {@code true} if all updates have been successful,
* {@code false} if one or more elements have been skipped
* because the foo list size limit has been reached.
*/
public boolean updateFoos(Map<BigInteger, List<String>> map)
{
boolean allUpdatesSuccessful = true;
for (Entry<BigInteger, List<String>> entry : map.entrySet())
{
List<String> newList = computeListForNumber(entry.getKey());
if (newList.size() > limit)
allUpdatesSuccessful = false;
else
entry.setValue(newList);
}
return allUpdatesSuccessful;
}
private List<String> computeListForNumber(BigInteger number)
{
// compute the new list
}
Lub nawet następujące (używając computeFoos(…)pierwszego przykładu, ale bez wyjątków):
/**
* Processes the passed numbers. An optimized algorithm will be used if any number
* produces a foo list of a size that justifies the additional overhead.
*
* @param numbers the numbers to process. Must not be {@code null}.
*/
public void process(Collection<BigInteger> numbers)
{
Map<BigInteger, List<String>> map = computeFoos(numbers);
if (isLimitReached(map))
processLarge(map);
else
processSmall(map);
}
private boolean isLimitReached(Map<BigInteger, List<String>> map)
{
return map.values().stream().anyMatch(list -> list.size() > limit);
}
Lub może to oznaczać coś zupełnie innego… ;-)