рефакторинг класса, чтобы быть более эффективным - PullRequest
Новая
       23

рефакторинг класса, чтобы быть более эффективным

0 голосов
/ 26 мая 2019

Я пытаюсь реорганизовать класс, который, как мне кажется, имеет несколько недостатков памяти и времени выполнения и ошибок. Можете ли вы помочь мне с этим или отметить мои предположения?

(например,

1. я понимаю, что метод tostring может использовать stringBuilder вместо конкатенации строк ...

2. Также, я думаю, лучше использовать хеш-таблицу вместо списка, чтобы получить результаты из m_strings в o (1) вместо o (n) (в remove_string_method)

Можете ли вы помочь мне одобрить это больше? Я также не уверен в отношении этих кастингов ... спасибо 

public class SomeClass
{
  private Date m_time;
  private String m_name;
  private List<Long> m_numbers;
  private List<String> m_strings;

  public MyClass(Date time, String name, List<Long> numbers, List<String> strings) {
        m_time = time; m_name = name; m_numbers = numbers; m_strings = strings;
  }

  public boolean equals(Object obj) { 
     if (obj instanceof MyClass) {
        return m_name.equals(((MyClass)obj).m_name);
     }
     return false;
  }

public String toString() {
   String out = m_name;
   for (long item : m_numbers) { 
      out += " " + item;
   }
   return out;
}

public void removeString(String str) {
    for (int i = 0; i < m_strings.size(); i++) { 
      if (m_strings.get(i).equals(str)) {
        m_strings.remove(i);
      }
   }
}

public boolean containsNumber(long number) {
    for (long num : m_numbers) {
     if (num == number) {
       return true;
     }
    }
return false;
}

public boolean isHistoric() {
    return m_time.before(new Date());
}
}

Ответы [ 2 ]

1 голос
/ 29 мая 2019

Я согласен с GhostCat , что вы не должны добавлять m_ к переменным-членам.

Но что более важно, вы должны прекратить изобретать велосипед.

Рассмотрим 

public boolean containsNumber(long number) {
    return numbers.contains(number);
}

Затем, с 

public void removeString(String str) {
    for(int i = 0; i < strings.size(); i++) { 
      if(strings.get(i).equals(str)) {
        strings.remove(i);
      }
   }
}

есть проблема в том, что при удалении элемента с индексом i последующие индексы сдвигаются на один, поэтому вы пропускаете вхождение после только что удаленного. С другой стороны, если вы думаете об использовании HashSet вместо этого, вы, возможно, не захотите поддерживать дублирующиеся элементы, и в этом случае вы можете остановиться после первого вхождения. Вы получаете все бесплатно, если не реализуете это самостоятельно. 

public void removeString(String str) {
    strings.remove(str);
}

удалит первый элемент (или только элемент, если вы не поддерживаете дубликаты) 

public void removeString(String str) {
    strings.removeAll(Collections.singleton(str));
}

удалит все вхождения str правильно.

Возможна ли замена List на Set, например HashSet, зависит от того, как вы собираетесь использовать данные, так как эти типы сбора имеют разную семантику. Поскольку вы вообще не используете данные в показанном коде, мы ничего не можем сказать по этому поводу.

Когда вы реализуете метод equals, у вас также должен быть согласованный метод hashCode 

@Override
public int hashCode() {
    return 1+name.hashCode();
}

@Override
public boolean equals(Object obj) { 
    return obj instanceof SomeClass && name.equals(((SomeClass)obj).name);
}
0 голосов
/ 26 мая 2019

Начните с именования: избегайте венгерских обозначений, поэтому отбросьте все префиксы m_!

И да, когда поиск содержащихся элементов является частой задачей, тогда вы, очевидно, используете реализацию Set, например HashSet.Это поможет с обоими наборами данных, именами и номерами!

...