Рефакторинг создания переменного числа планет в Java

Ответ 1

Начните с удаления дубликатов:

int zones = star.returnZones();
if (zones >= 1) {
    createPlanet(planet1, star);
}
if (zones >= 2) {
    createPlanet(planet2, star);
}
...

Вы можете видеть, что в примере, где zones == 2, он выполняет 2 блока, поэтому вы сохраняете ту же функциональность.

Затем вы можете видеть, что это фактически цикл. Начните с размещения планет в массиве:

Planet[] planets = new Planet[] {planet1, planet2, ...};
for (int i = 0; i < star.returnZones(); i++) {
    createPlanet(planets[i], star);
}

Ответ 2

Добавьте объекты планеты в массив, а затем вы можете написать:

Planet[] planets = {planet1, planet2, planet3, planet4, planet5};

for (int i = 0; i < star.returnZones(); i++) {
    this.createPlanet(planets[i], star);
}

Ответ 3

Один из способов сделать это - это инструкция switch с пошаговым поведением:

switch (star.returnZones()) {
    case 5: this.createPlanet(planet5, star); // fall-through
    case 4: this.createPlanet(planet4, star); // fall-through
    case 3: this.createPlanet(planet3, star); // fall-through
    case 2: this.createPlanet(planet2, star); // fall-through
    case 1: this.createPlanet(planet1, star); // fall-through
}

Так как переходное поведение достигается из-за отсутствия инструкции break, настоятельно рекомендуется добавлять поясняющие комментарии, как показано выше, чтобы читатель сразу понял, что отсутствующие операторы break преднамеренно.

Недостатком является то, что он создаст планеты в обратном порядке.

Альтернативой является цикл:

Planet[] planet={ planet1, planet2, planet3, planet4, planet5 };
int number = star.returnZones();
if(number>0 && number<=5) {
    for(int pIndex=0; pIndex<number; pIndex++)
        this.createPlanet(planet[pIndex], star);
}

Обратите внимание, что в любом случае вы должны подумать о том, что должно произойти со значениями вне диапазона. Если вы предполагаете, что этого никогда не произойдет, вы должны добавить код, который будет вызывать ошибку, если это допущение нарушено, чтобы немедленно обнаружить непоследовательные или поврежденные данные, а не продолжать с тем, что пользователь может сообщить как "странное поведение" ( не имея понятия о реальном источнике проблемы), например

switch (star.returnZones()) {
    default: throw new AssertionError("returnZones should be 1..5");
    case 5: this.createPlanet(planet5, star); // fall-through
    case 4: this.createPlanet(planet4, star); // fall-through
    case 3: this.createPlanet(planet3, star); // fall-through
    case 2: this.createPlanet(planet2, star); // fall-through
    case 1: this.createPlanet(planet1, star); // fall-through
}

или

Planet[] planet={ planet1, planet2, planet3, planet4, planet5 };
int number = star.returnZones();
if(number>0 && number<=5) {
    for(int pIndex=0; pIndex<number; pIndex++)
        this.createPlanet(planet[pIndex], star);
}
else throw new AssertionError("returnZones should be 1..5");

Ответ 4

В качестве альтернативы вы можете использовать статический метод с помощью varags, называемый createPlanets(PlanetCreator creator, Star star, Planet...planets).

И тогда все, что вам нужно будет сделать, это передать ваши планеты в виде массива или разделять запятыми (все равно массивом) и прокручивать их:

public static void createPlanets(PlanetCreator creator, Star star, Planet...planets) {
    for (Planet p: planets) {
        creator.createPlanet(p, star);
    }
}

Ответ 5

Другие ответы верны, что повторяющуюся работу, подобную этой, можно обрабатывать аккуратно с помощью цикла.

С Java 8 есть еще один, возможно, даже опрятный вариант - использование Stream API для объявления повторяющейся работы в функциональном стиле. Вы можете получить все, что сделано в однострочном пространстве.

Arrays.asList(planet1, planet2, planet3 /*...etc*/)
  .stream()
  .limit(star.returnZones())
  .forEach((planet) -> this.createPlanet(planet, star));

Обратите внимание, что первые три строки есть только для иллюстрации установки потока длинных n планет - это может и должно выполняться динамически другим способом, а не быть статически определенным для этого, чтобы упростить дальнейшее.

Ответ 6

Существует другое решение, подобное первой части ответа @njzk2: switch/case с провалом:

int zones = star.returnZones();
switch (zones ) {
 case 5:
    createPlanet(planet5, star);
 case 4:
    createPlanet(planet4, star);
 case 3:
    createPlanet(planet3, star);
 case 2:
    createPlanet(planet2, star);
 default:
    createPlanet(planet1, star);   
}

Ответ 7

Заявление о проблеме иногда само по себе направлено на решение. В вашем случае вам нужно повторить аналогичную задачу. Для каждой задачи есть другой planet, но опять же он следует за порядком. Такая проблема может быть легко решена путем циклизации.

Простое решение может помещать объекты planet1, planet2,... planet3 в ArrayList. Затем, используя цикл for, повторяющий для star.returnZones() раз и вызывающий createPlanet в каждой итерации, передавая элемент ith из списка массивов для итерации ith.

Ответ 8

Небольшое отклонение от ответа на массивы:

List<Planet> planets = Arrays.asList(planet1, planet2, planet3 /*etc*/);
for (Planet p : planets.subList(0, start.returnZones()) {
  createPlanet(p, star);
}