Single responsibility

0

Nie czaję tego postulatu. Mam wrażenie, że każda klasa powinna mieć po 1 metodę by miała tylko 1 powód do zmiany...

Zrobiłem klasę zliczającą ilość wystąpień słów w tekście:

import java.util.Map;
import java.util.TreeMap;

public class WordsCounter {

	Map<String, Integer> wordsMap = new TreeMap<>();
	String[] singleWords = {};

	public void add(String text) {
		splitWords(text);
		calcWords();
	}

	public void splitWords(String text) {
		singleWords = text.split(" ");
	}

	public void calcWords() {
		Integer value;
		for (String x : singleWords) {
			value = wordsMap.get(x);
			if (value == null) {
				wordsMap.put(x, 1);
			} else {
				wordsMap.put(x, ++value);
			}
		}
	}

	public Map<String, Integer> getWordsCounted() {
		return wordsMap;
	}

}

czy jest tu złamana zasada single responsiblity i powinienem rozdzielić to na 2 klasy?
1 klasa: zliczająca słowa i przechowująca je?
2 klasa wyłuskującą pojedyncze słowa z tekstu i przechowująca je?

1

Tak, powinieneś to rozdzielić do dwóch odrębnych klas, dokładnie jak opisałeś, ponieważ w aktualnej formie Twoja klasa ma dwie funkcjonalności: dzielenie wyrazów oraz ich grupowanie.

Btw, taki protip:

for (String word: singleWords) {
	wordCount = wordsMap.containsKey(word) ? wordsMap.get(word) : 0;

	wordsMap.put(word, wordCount + 1);
}

Nie programuję w Javie, ale jakoś tak to powinno wyglądać porządniej imo.

2

A IMO nie trzeba tutaj nic rozdzielać. Średnio widzę sens tworzenia klasy do dzielenia wyrazów, która jedyne co robi to wywołuje Javowy .split() na stringu.
Zastanawia mnie tylko dlaczego metoda splitWords() jest publiczna i po co w ogóle metoda o dziwnej nazwie add()?

0

0

Jest publiczna, bo prywatną ciężko testować. Metoda nazywa się add, bo będzie wywoływana wiele razy na różnych tesktach i za każdym wywołaniem będą się dodawać policzone słowa do mapy, tworząc mapę z wielu tekstów.

W sumie mógłbym zrobić tak (tylko, że teraz jak będę chciał rozbudować splita np. by wywalał znaki interpunkcyjne to zacznie się robić bajzel:

import java.util.Map;
import java.util.TreeMap;
 
public class WordsCounter {
 
    Map<String, Integer> wordsMap = new TreeMap<>();
   
 
    public void add(String text) {
         String[] singleWords = text.split(" ");
        calcWords(singleWords);
    }
 
    public void calcWords(String[] singleWords) {
        Integer value;
        for (String x : singleWords) {
            value = wordsMap.get(x);
            if (value == null) {
                wordsMap.put(x, 1);
            } else {
                wordsMap.put(x, ++value);
            }
        }
    }
 
    public Map<String, Integer> getWordsCounted() {
        return wordsMap;
    }
 
}

czy teraz też można mówić o naruszeniu zasady single responsibility?

0

A dlaczego nie:

import java.util.Map;
import java.util.TreeMap;
 
public class WordsCounter { 
    public Integer calcWords(String text) {
         return text.split(" ").length;
    }
}

I wtedy Twoja klasa robi to co mówi, czyli liczy słowa :)

0

ja chcę zrobić table() a nie length.

0
  1. Bez sensu robić z metod prywatnych publiczne, tylko po to, żeby były testowalne... Testujesz metody publiczne.
  2. Nazwa "add" taka sobie, "dodaj, ale co?". Nie lepiej byłoby nazwać tę metodę zgodnie z tym co robi? np. calculateWordsInText?

Zakładając, że skończysz z klasą:

public class WordCounter {
  void calculateWordsInText(String text);
  Map<String,Intger> getWordsStatistics();
}

To ile będziesz miał powodów do jej modyfikacji? Według mnie nie ma naruszenia SRP w takim przypadku.

0

a po co dodawać coś do add skoro klasa mówi o tym co tam się dodaje i po co? W Map masz np. put w List też masz add.

1 użytkowników online, w tym zalogowanych: 0, gości: 1