Pytania juniora odnośnie dobrych praktyk ;) To czego nie ma w książkach ;)

1. Jak realizować testowanie, gdy metoda obraca sporymi pakietami danych? Czy w przypadku testowym stworzyć minimalny obraz kilku kolekcji, które są potrzebne? (nawet jeśli potrzeba na to z 20-30linii kodu?) Nie da się chyba uniknąć takich sytuacji?

2. Metody robiące robotę pomocniczą dla metod publicznych zwykle oznaczam jako private. Pojawia się wtedy problem z testowaniem tych metod. Generalnie samo testowanie metody publicznej testuje też te prywatne, ale bardziej zasadne byłoby testowanie tych metod odrębnie (co ułatwia też szukanie ewentualnych błędów). Na jednej z rekrutacji zjechano mnie za to, że jedną z metod zrobiłem publiczną by móc ją testowować, a wcale nie było silnych przesłanego by musiałą być private.

3. Czy każda metoda powinna absolutnie coś zwracać? Czy akceptowalne są metody które zmieniają stan obiektu (wartości pola prywatne), a klasa (obiekt) posiada odrębne metody zapewniają dostęp do tych pół? Utrudnia to testowanie (choć nie uniemożliwia), ale w pewnych okolicznościach funkcjonalnych pasują mi bardziej takie rozwiązania.

4. Jak organizujecie sobie kod aplikacji w pakietach? Staram się rozdzielać główne klasy, logikę biznesową i klasy związane z danymi, ale już przy kilkunastu klasach mam wrażenie, że robi się bałagan. Przy aplikacjach webowych springa problem znika, bo tam w zasadzie wszystko się samoorganizuje, ale po dodaniu rozbudowanej logiki biznesowej też robi się bałagan.

5. W jakich okolicznościach uzasadnione jest używanie interfejsów (poza oczywistymi gdy chodzi o obiekty polimorficzne). Wiem, że niektórzy zalecają rozpoczęcie prac nad aplikacją od rozpisania interfejsów

6. Na koniec pytanie konkretne ;) Mam prostą aplikację Spring-Boot. W głównym pakiecie mam klasę startową z "SpringApplication.run(Application.class);", jest to też klasa konfiguracyjna. Aplikacja korzysta z automatycznych repozytoriów Spring Data JPA. Niestety wszystko mi działa tylko jeśli klasy encji i interfejsy repozytoriów są w tym samym pakiecie. Teoretycznie powinno wystarczyć dodanie adnotacji:
   @EnableJpaRepositories(basePackages = "repository")
   , ale nie działą. Próbowałem także z @EntityScan, bez poprawy.
   Gdzie mogę robić coś źle, bo nie wierzę, że nie da się wyrzucić klas związanych z bazami danych do innego pakietu niż ten z klasą konfiguracyjną.

Z góry dzięki za pomoc. Będę wdzięczny za odpowiedzi lub wskazówki choć do jednoego pytania ;)

Tak z ciekawości ;) Z jakich źródeł uczyliście się tego co w punktach 1-5 ?

1. To znak, że metoda robi za dużo i trzeba ją podzielić.
2. Nie testujesz metod, a zachowania.
3. Czemu by nie.
4. Patrz Clean Architecture
5. Chociażby po to by najpierw zdefiniować sobie zachowania, napisać testy, a potem użerać się z implementacją → TDD
6. Poka sowe... znaczy się kod.

Do tych pierwszych też pokaż kod, przynajmniej pomożemy ogarnąć jak to sensownie podzielić.

1. Poniżej metoda która w argumencie ma listę portów lotniczych, zwraca listę portów do których są połączenia z portów podanych w argumencie.
   By metoda działała potrzebuje dostępu do kolekcji z mapą połączeń (connectionsByOrigin). Nie przetestuję metody bez tej kolekcji, choćby uproszczonej, a by wykorzystać tę kolekcję potrzebuję też referencji do obiektu klasy Airports który przechowuje dane dotyczące portów (w tym przypadku kod i nazwa miasta). Dla potrzeby testu bez problemu da się dorobić szybko kilka pozycji w tych kolekcjach by wszystko działało, ale czy tak się robi ?

	public Set<String> calculateNeighbors(Set<String> initAirportNames) {
		Set<String> neighbors = new TreeSet<>();
		for(String initAirportName : initAirportNames) {
			String initAirportCode = airports.getAirportCodeByName(initAirportName);
			List<Connection> connections = connectionsByOrigin.get(initAirportCode);
			for (Connection connection : connections) {
				String connectionName = airports.getAirportNameByCode(connection.getDestination());
				neighbors.add(connectionName);
			}
		}
		return neighbors;
	}

Powyżej zapomniałem się zalogować ;)

2. Odnośnie drugiego problemu, testowania. Metoda z pkt 1 mogłaby być private, bo korzysta z niej inna metoda (poniżej kod), która oblicza połączenia z uwzględnieniem większej ilości segmentów (przesiadek) i jest elastyczna (2-4 segmentów, czyli 1-3 przesiadki). Ta metoda jest już publiczna. Mogę ją testować, ale wydaje mi się zasadne by testować też tę z pkt 1.
   Metoda korzysta z pól obiektu:* routeNr, size* oraz kolekcji routeLineList w której pozostaje efekt pracy metody (później korzysta z tego inna klasa).

	public void recalculate(Set<String> initAirportNames) {
		int positionL = routeNr;
		int positionR = routeNr;
		Set<String> neighbours = initAirportNames;
		routeLineList.set(routeNr,neighbours);
		while (isNeighbours(positionL,positionR)) {
			positionL--;
			positionR++;
			Set<String> newNeighbours = calculateNeighbors(neighbours);
			if (positionL>=0) routeLineList.set(positionL,newNeighbours); 
			if (positionR<size) routeLineList.set(positionR,newNeighbours);
			neighbours = newNeighbours;
		}
	}

4. Google niewiele wyrzuca o Clean Architecture. Jest jakieś dobre źródło wiedzy ?
   Samo to https://8thlight.com/blog/uncle-bob/2012/08/13/the-clean-architecture.html to mało i tyle mniej więcej mniej. Mam wrażenie, że robi mi się bałagan w kodzie wewnątrz poszczególnych warstw.

5. Aplikacja odczytuje z publicznego API dane portów i połączeń w formacie JSON i zapisuje wszystko do bazy. Wszystko działa pod warunkiem, że jest w jednym pakiecie:
   Jeśli oddzielę AirportsData.java i AirportsDataRepository.java do odrębnego package (nazwa "repository") to aplikacja nie działa.

Application.java

@Configuration
@SpringBootApplication
@EnableJpaRepositories  //(basePackages = "repository") - nie działa
@EnableAutoConfiguration
@ComponentScan
public class Application implements CommandLineRunner {

	@Autowired
	private DataFetch dataFetch;
	
	public static void main(String args[]) {
		SpringApplication.run(Application.class);
	}

	@Override
	public void run(String... args) throws Exception {
		dataFetch.initAirports();
		dataFetch.saveAirports();
		dataFetch.initConnectionScan();
	}
}

AirportsDataRepository.java

package app;

import java.util.List;

import org.springframework.data.repository.CrudRepository;
import org.springframework.data.repository.query.Param;
import org.springframework.stereotype.Repository;

@Repository
public interface AirportsDataRepository extends CrudRepository<AirportsData, Long> {
	List<AirportsData> findByCityCode(@Param("cityCode") String cityCode);
}

AirportsData.java

package app;

import java.io.Serializable;
import javax.persistence.*;

@Entity
@NamedQuery(name="AirConnection.findAll", query="SELECT a FROM AirConnection a")
public class AirConnection implements Serializable {
	private static final long serialVersionUID = 1L;

	@Id
	@GeneratedValue(strategy=GenerationType.AUTO)
	private int id;
	
	private String airlineCode;

	private String destination;

	private String stops;

	//bi-directional many-to-one association to Airportsdata
	@ManyToOne
	private AirportsData airportsData;

	public AirConnection() {
	}
	
	public AirConnection (AirportsData origin, Connection connection){
		this.airportsData = origin;
		this.destination = connection.getDestination();
		this.stops = connection.getStops();
		this.airlineCode = connection.getAirlinecode();
	}

	public int getId() {
		return this.id;
	}

	public void setId(int id) {
		this.id = id;
	}

	public String getAirlineCode() {
		return this.airlineCode;
	}

	public void setAirlineCode(String airlineCode) {
		this.airlineCode = airlineCode;
	}

	public String getDestination() {
		return this.destination;
	}

	public void setDestination(String destination) {
		this.destination = destination;
	}

	public String getStops() {
		return this.stops;
	}

	public void setStops(String stops) {
		this.stops = stops;
	}

	public AirportsData getAirportsData() {
		return this.airportsData;
	}

	public void setAirportsData(AirportsData airportsData) {
		this.airportsData = airportsData;
	}
	
}

Przekombinowałeś i to na wielu poziomach.

1. w tej metodzie wykonujesz za wiele rzeczy na raz. Masz funkcjonalności „wysokiego” poziomu – pobranie listy połączeń, jak i „niskiego” poziomu – realizacja tej operacji:

String initAirportCode = airports.getAirportCodeByName(initAirportName);
List<Connection> connections = connectionsByOrigin.get(initAirportCode);

można zamienić na:

Collection<Connectionis> getConnectionsByAirportName(String initAirportName){
	String initAirportCode = airports.getAirportCodeByName(initAirportName);
	return connectionsByOrigin.get(initAirportCode);
}

podobnie z:

for (Connection connection : connections) {
	String connectionName = airports.getAirportNameByCode(connection.getDestination());
        neighbors.add(connectionName);
}

które można zamienić na:

Set<String> neighborsFor(Collection<Connectionis> conn){
	return conn.stream().map(Connection::getDestination).map(Airports::getAirportNameByCode).collect(TreeSet::new, TreeSet::add, TreeSet::addAll);
}

wtedy w twoim kodzie można już wołać dwie metody zamiast kombinować z kilkoma poziomami zagnieżdżenia. Łatwiej będzie też podstawić dublera do testów. Niekoniecznie mocka, ale np. małą kolekcję.

1. Jesli metoda jest trudna w testowaniu to jest zle napisana. Jesli setup testu jest mega zlozony to metode najpewniej trzeba podzielic
2. jw, nie testuj "metod" tylko funkcjonalnosci. Jednoczesnie jesli jakies prywatne metody sa zbyt zlozone to znaczy ze pewnie trzeba zrobic z nich publiczne (albo package) metody osobnego obiektu/serwisu/utila
3. Nie bo czasem nie ma to sensu. Niemniej jesli cala twoja aplikacja opiera sie o takie voidowe metody to cos jest nie tak. Generalnie duzo wygodniej pracuje sie z obiektami immutable i stateless a z nimi innej mozliwosci nie ma - trzeba zwracac wyniki.
4. Dziwne pytanie. Tam gdzie sa potrzebne. Tam gdzie masz jakies wspolne zachowania. Tam gdzie chcesz jasno zdefiniowac jakies API (np. na styku modulow).
5. Brakuje ci w ComponentScan informacji o tym co ma byc skanowane.

rob_sad napisał(a):

2. Metody robiące robotę pomocniczą dla metod publicznych zwykle oznaczam jako private. Pojawia się wtedy problem z testowaniem tych metod. Generalnie samo testowanie metody publicznej testuje też te prywatne, ale bardziej zasadne byłoby testowanie tych metod odrębnie (co ułatwia też szukanie ewentualnych błędów).

I wybitnie utrudnia utrzymywanie testów, bo przy takim podejściu zmiana każdej metody (czy np. zastąpienie jednej grupy metod inną grupą metod) będzie wymuszać naniesienie zmian w testach.

W kodzie "prywatnym" zawieramy szczegóły implementacyjne, które zewnętrznego świata nie powinny obchodzić. Dzęki enkapsulacji możemy sobie te szczegóły dowolnie zmieniać nie wymuszając przeróbek w pozostałym kodzie. To tyczy się nie tylko testów.

Testy stanowią dla nas asekurację, bo potwierdzają, że bez względu na nasze grzebanie w bebechach, publiczne API naszego kodu wciąż zachowuje się tak samo. I to jest rozwiązanie optymalne.