2013-04-23 11 views
11

Mam dwie podobne metody. Jeden z nich drukuje coś, a jeden z nich coś oszczędza. Jak widać, istnieje wiele duplikatów kodu. Jak powinienem to zmienić i usunąć to powielanie?Jak usunąć duplikat z mojego kodu?

public static void printSomething(List<String> list) { 
    for (String item : list) { 
     if (item.contains("aaa")) { 
      System.out.println("aaa" + item); 
     } 
     if (item.contains("bbb")) { 
      System.out.println("bbb" + item); 
     } else { 
      System.out.println(item); 
     } 
    } 
} 

public static Map<String, String> getSomething(List<String> list) { 
    Map<String, String> map = new HashMap<String, String>(); 
    for (String item : list) { 
     if (item.contains("aaa")) { 
      map.put("aaa", item); 
     } 
     if (item.contains("bbb")) { 
      map.put("bbb", item); 
     } else { 
      //do nothing 
     } 
    } 
    return map; 
} 

UPDATE:

kod został zaktualizowany, aby rozwiązać problem, gdy metoda nie jest dokładnie podobna

+0

To zależy ... dlaczego masz te 2 metody ... można dzielić metody wywołujące – Frank

+0

Prawdopodobnie mógłbyś użyć zwracanej wartości 'getSomething()', aby wykonać wydruki w 'printSomething()'; Poszedłbym tą drogą, aby uniknąć metod, które nie zwracają niczego (które nie są ustawiaczami c). – adrianp

+0

@HighPerformanceMark przepraszam za pomyłkę. Mam na myśli duplikację – hudi

Odpowiedz

3

ogólnego interfejsu działanie, które mają działanie metody (T t) może zmniejszyć kod.

public interface Action<E> { 
     void action(E e); 
} 

Przykład:

public static void forEach(List<String> list, Action <String> action) { 
    for(String s : list){ 
      action.action(s); 

} 

Teraz wystarczy 2 różne implementacje działania.

Możesz użyć typów annonymous, jeśli nie chcesz tworzyć zajęć.

Jeśli znasz C#, to jest podobne do lambdas.

edit:

Korzystanie typ annonymous:

public static Map<String, String> getSomething(List<String> list) { 
    final Map<String, String> map = new HashMap<String, String>(); 
    forEach(list, new Action<String>() { 
     @Override 
     public void action(String e) { 
      if (e.contains("aaa")) { 
       map.put("aaa", e); 
      } 
      if (e.contains("bbb")) { 
       map.put("bbb", e); 
      } else { 
       // do nothing 
      } 
     } 
    }); 
    return map; 
} 

Tworzenie klasy:

public static Map<String, String> getSomething2(List<String> list) { 
    final Map<String, String> map = new HashMap<String, String>(); 
    forEach(list, new ListToMapAction(map)); 
    return map; 
} 


public class ListToMapAction implements Action<String> { 

    Map<String, String> map; 

    public ListToMapAction(Map<String, String> map) { 
     this.map = map; 
    } 

    @Override 
    public void action(String e) { 
     if (e.contains("aaa")) { 
      map.put("aaa", e); 
     } 
     if (e.contains("bbb")) { 
      map.put("bbb", e); 
     } else { 
      // do nothing 
     } 
    } 

} 
+0

Przepraszam, ale nie rozumiem twojego przykładu. Twoja metoda nic nie zwraca, więc jak mam pobrać mapę? – hudi

+0

edytowane z pełną implementacją –

+0

kiedy po prostu drukujesz coś? Myślę, że będzie jeszcze trochę powielania w działaniu metody – hudi

2

w języku programowania z funkcji pierwszej klasy, którą przechodzą wokół funkcji jako parametr wskazujący co chcesz zrobić w pętli (na przykład patrz aktualizacja, poniżej). Java będzie miała lambdas w wersji 8, ale nie są one w stanie wykonać tej pracy.

W bieżącym stanie Java będziesz musiał zadowolić się czymś brzydszym - na przykład przekazując dodatkowy parametr do metody; lub można przejść wokół anonimowych klas wewnętrznych, które implementują interfejs, ale IMHO to jeszcze brzydsze niż to, co mam zamiar zaproponować:

static void printSomething(List<String> list, boolean print) 

Jeśli print jest true następnie wydrukować wewnątrz pętli, w przeciwnym razie dodać do Map. Oczywiście w pętli musisz dodać kilka if s w celu sprawdzenia tego warunku, a na początku jeszcze jeden if, aby ustalić, czy należy zainicjować Map. Tak czy inaczej, metoda zwraca Map, ale Map może być null dla przypadku drukowania. To, co mam na myśli:

static Map<String, String> processSomething(List<String> list, boolean print) { 

    Map<String, String> map = null; 
    if (!print) 
     map = new HashMap<String, String>(); 

    for (String item : list) { 
     if (item.contains("aaa")) { 
      if (print) 
       System.out.println("aaa" + item); 
      else 
       map.put("aaa", item); 
     } 
     if (item.contains("bbb")) { 
      if (print) 
       System.out.println("bbb" + item); 
      else 
       map.put("bbb", item); 
     } else if (print) { 
      System.out.println(item); 
     } 
    } 

    return map; 

} 

UPDATE

Na przykład w Pythonie - co pozwala przejściu funkcji jako parametry, to jak chcesz rozwiązać problem w eleganckim mody:

def processSomething(lst, func): 
    result = None 
    for item in lst: 
     if 'aaa' in item: 
      result = func(item, 'aaa', result) 
     elif 'bbb' in item: 
      result = func(item, 'bbb', result) 
     else: 
      result = func(item, '', result) 
    return result 

def printer(item, key, result): 
    print key + item 

def mapper(item, key, result): 
    if not result: 
     result = {} 
    if key: 
     result[key] = item 
    return result 

Zobacz, jak to działa:

processSomething(['aaa', 'bbb', 'ccc'], printer) 
=> aaaaaa 
    bbbbbb 
    ccc 

processSomething(['aaa', 'bbb', 'ccc'], mapper) 
=> {'aaa': 'aaa', 'bbb': 'bbb'} 
+3

to bardzo brzydki przykład. Imię tej metody wydrukować lub uzyskać? Po drugie nadal masz pustkę, więc nic nie zwracasz. – hudi

+0

Nazwa może być czymś ogólnym. Metoda musi zawsze zwracać mapę, nawet jeśli jest ona pusta. Dobra rada: –

+0

. I zawsze możesz przekazać implementację interfejsu - wzorca delegata. –

7

ASSU ming kolejność których println z "aaa" i "bbb" pojawić nie ma znaczenia, można wymienić realizację printSomething z

public static void printSomething(List<String> list) { 
    Map<String, String> map = getSomething(list); 
    for(Map.Entry<String, String> entry : map) { 
    System.out.println(entry.getKey() + entry.getValue()); 
    } 
} 
+0

+1. Zakłada to, że kolejność wydruków nie ma znaczenia. – Keppil

+0

Dobrze - chciałem to napisać, dziękuję za wskazanie. – tehlexx

+0

hm thx to zadziała, jeśli metoda będzie taka sama, ale co jeśli druk będzie zawierał: ... else {System.out.println ("bbb" + item); } i otrzymasz ... else {// doNothing} – hudi

Powiązane problemy