Решение второго примера кампании Anti-If - PullRequest
Новая
       23

Решение второго примера кампании Anti-If

0 голосов
/ 31 января 2020

1. Моя попытка

Первый пример плохого кода на веб-сайте Anti-If Campaign кажется мне довольно тривиальным и стандартным. Решение, как указывалось во множестве книг, других вопросов и веб-сайтов StackOverflow, - это полиморфизм.

Однако, второй пример с сервером мне не кажется таким простым, что свидетельствует о том, что я не очень хороший программист, наверное. Я хотел бы, чтобы кто-то подтвердил, если мои попытки решения верны; или, если нет, дайте лучше. (Мои баллы пронумерованы, так что ответ может более легко ссылаться на них, это , а не иерархический порядок.)

  1. Даже если есть только один if-null условие выделено, в этом примере множество таких случаев. Я думаю, что лучший способ решения проблемы такого типа - использовать Null Object Pattern .
  2. Вероятно, самая большая проблема с этим фрагментом кода - это даже не сами if, но огромное количество вложений, из которых мы можем встретить до 5 уровней.
  3. Условное if (getArea() == 1) не кажется мне таким уж плохим, если оно только в этой части кодовой базы. Но это, конечно, не раскрывает столько смысла и намерений для читателя. Что означает 1? Это достаточная оговорка? Тогда, возможно, было бы лучше заменить его на что-то вроде if (validArea()). Он также не привязан ни к какому объекту, поэтому я не могу легко установить отношения sh.
    1. Я думаю, что если предложение может также извлечь выгоду из Фабрики или Шаблонного Метода, в зависимости от того, каковы отношения объектов - мне не очень понятно, какой будет правильный выбор из этого кусок кода, но, возможно, кто-то более опытный может иметь лучшее решение

2. Ссылочный код

Для справки, это фрагмент кода (Java) на веб-сайте (есть некоторая странность с отступом и комментированием строки else, я думаю): 

public ArrayList eseguiAnalisi() {
  ArrayList listaSegnalazioni = new ArrayList();

  List domande = null;
  List risposte = null;
  Domanda domanda = null;

  DAOReques req = new DAOReques();
  DAOArea ar = new DAOArea();
  ar.setIdQuestionario(getIdQuestionario());

  boolean quizDomandaNonForzata = false; // warning 7

  try {
    //set Aree e
    if (getArea() == 1) { // Highlight #1
      List aree = ar.getAree();
      Area area = null;

      for (int i = 0; i < aree.size(); i++) {
        area = (Area) aree.get(i);
        domande = req.getDomande(getIdQuestionario(),area.getIdArea());
        if (domande != null) {
          for (int j = 0; j < domande.size(); j++) {
            domanda = (Domanda) domande.get(j);
            risposte = req.getRisposteDomanda(getIdQuestionario(),domanda.getIdDomanda());
            if (risposte != null)
            domanda.setRisposte(risposte);
          }
        }
      }

      area.setDomandeArea(domande);

      if (aree != null) // Highlight #2
      setAreeDomande(aree);
    } //else { // I think there was an indentation mistake in the website...
      // set ListaDomande
      domande = req.getDomande(getIdQuestionario());


      for (int i = 0; i < domande.size(); i++) {
        domanda = (Domanda) domande.get(i);
        risposte = req.getRisposteDomanda(getIdQuestionario(),domanda.getIdDomanda());
        if (risposte != null)
        domanda.setRisposte(risposte);
      }
      if (domande!=null)
      setDomande(domande);
  //}
  } catch (DAOException de) {
    de.printStackTrace();
  }
...