jdbc metody związane z wypożyczaniem i oddawaniem książek, redundancja? CZYTAJ DO KOŃCA ;)

Question

Witam serdecznie! Proszę o poradę jak mam zrobić ten program :P a raczej jak rozwiązać mój problem.

Posiadam metody:

-księgozbiór

-lista czytelników

-lista wypożyczonych ksiązek

-wypożycz książkę (zmienia wartość zmiennej czyJest książki na false)

Mam dwie kolidujące ze sobą operacje (zaraz pokaże dlaczego)

wypożyczenie książki (lista czytelników, księgozbiór, wypożycz książkę-->jedna metoda wywołuje drugą)

-pokazanie księgozbioru (i tu kicha bo ona wywołuje metodę wypożycz książkę)

-pokazanie listy czytelników-->analogicznie.

Byłoby prościej w mainie to robić i zarządzać osobno tymi metodami, ale chciałem i tu UWAGA:

mieć możliwość reagowania na wybranie (np przy oddawaniu książki) wybrania złej książki czyli indeksu którego nie ma na liście i żeby można było wybrac taki numer jaki jest na liście. Wpadłem na pomysł wrzucenia do resultsetu id_czytelnika/id_ksiązki a potem do listy. Potem sprawdzam czy liczba znajduje sie w liscie (countains) i git. Tylko że pytanie: można zrobić to prościej? Jak to oddzielić od metody tylko wyświetlającej? Zrobić metodę tylko wyświetlającą liste książek/czytelników (będzie robiła to samo tylko bez dodatkowych opcji (przeskakiwania między metodami)) tylko wiąże się to z powtarzaniem kodu... Jak można to zrobić? Raz jeszcze zależy mi na tym żeby wtedy kiedy trzeba coś wybrać móc wybrać tylko taką liczbę, jakie indeksy mam pokazane... Wrzucam cały kod bo może będzie potrzeba coś więcej zobaczyć.. Mój pomysł? Powtarzanie metod które będą do innych celów :(

https://pastebin.com/z8ed2jnU

https://pastebin.com/L9RLNUT7

Answer 1

Zadałeś pytanie w strasznie niejasny sposób, więc zacznę od krytyki kodu a potem bd myślał o co Ci chodzi.

Stosujesz bardzo niejasne nazewnictwo.

    Scanner ksiazki = new Scanner(System.in);

Czemu skaner nazywa się książki?

Boolean ok = false;

Bardzo deskryptywna nazwa zmiennej, wszystko jasne na pierwszy rzut oka :D Po co klasa opakowująca, czemu nie zwykłe boolean?

insertKsiazka()

skoro już piszemy po polsku to czemu nie dodajKsiazke()?

księgozbiór()

metoda o takiej nazwie wyswietla dostepne ksiazki i umozliwia wypozycznie? Coś tu jest nie tak.

listaCzytelnikow()

Spodziewałbym się, że metoda o takiej nazwie albo zwróci listę czytelników, albo ja wypisze u Ciebie nic nie zwraca więc pewnie ja wypisuje (wtedy powinna się nazywać wyswietlListeCzytelnikow()). A ta metoda wypisuje liste czytelników a potem pozwala wyszukac konkretnego i ło matko i córko na końcu wywołuje metode księgozbiór, czyli ta metoda robi wszystko.

Kolejna sprawa to te do while w metodach. Po co do while a nie zwykłe while, a po drugie - w takiej aplikacji powinna być raczej jedna nieskończona pętla while, w której wywoływane metody się kończą. Jeśli wywoływanie metod biblioteki blokuje program nieskończoną pętlą to taka biblioteka praktycznie nie ma zastosowania, bo ktoś kto z niej korzysta od pewnego momentu nie moze juz nic zrobić.

Co do wykorzystania bazy danych to brakuje Ci odpowiedzi na to pytanie:

https://www.youtube.com/watch?v=sk6BzH_Htqg

:P. Skoro już używasz SQL'a to użyj jakichś relacji :P. Tabela wypozyczenia nie ma jakiegokolwiek polaczenia z ksiazkami i czytelnikami. Mozna tam wpisać dowolne id.

 

Nie do końca rozumiem pytanie. Ale myślę, że może Ci pomóc dzielenie programu na mniejsze metody:

Zrobić metodę tylko wyświetlającą liste książek/czytelników (będzie robiła to samo tylko bez dodatkowych opcji (przeskakiwania między metodami)) tylko wiąże się to z powtarzaniem kodu...

W jaki sposób? Właśnie przy pomocy takiej metody unikniesz potwarzania kodu.

Comments

Dzięki za wyczerpującą odpowiedź :) W międzyczasie zrobiłem już wszystko (prawie xd) Dzięki też za tą krytykę kodu bo faktycznie przegiąłem xD 

scanner nazywa się książki bo miały być osobne do książek i do czytelników potem stwierdziłem po jaki uj ale nazwa została (do zmiany)

pewnie że bardzo opisowa coż bardziej opisuje zmienna jak słowo "ok" :D Nie za bardzo rozumiem o co Ci chodziło z tą klasa opakowującą??

zrobiłem teraz dwie metody księgozbiór() teraz wygląda jak powinien a doWypozyczenia() tak jak wczesniej ten "uniwersalny" ksiegozbior

listaCzytelnikow() hmmmm nie pomyslalem nad tym faktycznie zla nazwa.. Nie wiedzialem za bardzo jakiej petli uzyc dlatego zrobilem do..while(w sumie z while tez mozna xd) no i nieskonczona bo niby jaki warunek? ktos kiedys pisal zeby nie stosowac break i continue no ale co poradzic tutaj pasuje idealnie. 

Teraz po wywołaniu metody normalnie mam break i juz nie ma nieskonczonej. Nie potrafie wlasnie zrobic relacji :/ teraz czytam cos na temat JOIN (może mi to pomoze :D)

z tym powtarzaniem kodu wlasnie chodzilo mi o to czy da sie zrobic inaczej niz to co teraz zrobilem czyli podobne metody (jedna dokokszona wywolaniami itp) jak ksiegozbior() & do Wypozyczenia() oraz listaCzytelnikow() & wypozyczajacy() :D

Tak to teraz wyglada: (wszystko dziala nazwy metod zmienione przed chwilą :D): (uwagi mile widziane :D)

 

package biblioteka;

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.util.LinkedList;
import java.util.List;
import java.util.Scanner;

public class Biblioteka
{
	public static final String DRIVER = "org.sqlite.JDBC";

	public static final String URL = "jdbc:sqlite:biblioteka.db";

	private Connection connection;
	private Statement statement;
	Scanner scanner = new Scanner(System.in);

	public Biblioteka()
	{
		try
		{
			Class.forName(DRIVER);
			connection = DriverManager.getConnection(URL);
			statement = connection.createStatement();

		} catch (ClassNotFoundException e)
		{
			System.err.println("Nie udało się nawiązać połączenia z bazą danych !\n");
		} catch (SQLException e)
		{
			System.err.println("Nie udało się załadowac sterownika JDBC !\n");
		}
		stworzTabele();
	}

	public void stworzTabele()
	{
		String createCzytelnicy = "CREATE TABLE IF NOT EXISTS czytelnicy(id_czytelnika INTEGER PRIMARY KEY AUTOINCREMENT, imie VARCHAR(20), nazwisko VARCHAR(20), login VARCHAR(10) UNIQUE)";
		String createKsiazki = "CREATE TABLE IF NOT EXISTS ksiazki(id_ksiazki INTEGER PRIMARY KEY AUTOINCREMENT, tytul VARCHAR(20), autor VARCHAR(20), czyJest BOOLEAN)";
		String createWypozyczenia = "CREATE TABLE IF NOT EXISTS wypozyczenia(id_wypozycz INTEGER PRIMARY KEY AUTOINCREMENT, id_czytelnika, id_ksiazki, FOREIGN KEY(id_czytelnika) REFERENCES czytelnicy(id_czytelnika), FOREIGN KEY(id_ksiazki) REFERENCES ksiazki(id_ksiazki))";
		try
		{
			statement.execute(createCzytelnicy);
			statement.execute(createKsiazki);
			statement.execute(createWypozyczenia);
			statement.close();
		} catch (SQLException e)
		{
			System.err.println("Błąd przy tworzeniu tabel !\n");
			System.err.println(e);
		}
	}

	public void dodajCzytelnika(String imie, String nazwisko, String login)
	{
		try
		{
			PreparedStatement preparedStatement = connection
					.prepareStatement("INSERT INTO czytelnicy VALUES(NULL, ?, ?, ?)");
			preparedStatement.setString(1, imie);
			preparedStatement.setString(2, nazwisko);
			preparedStatement.setString(3, login);
			preparedStatement.execute();
			preparedStatement.close();
			System.out.println("\nCzytelnik dodany !\n");
		} catch (SQLException e)
		{
			System.err.println(e);
		}
	}

	public void dodajKsiazke(String tytul, String autor)
	{
		try
		{
			PreparedStatement preparedStatement = connection
					.prepareStatement("INSERT INTO ksiazki VALUES(NULL, ?, ?, true)");
			preparedStatement.setString(1, tytul);
			preparedStatement.setString(2, autor);
			preparedStatement.execute();
			preparedStatement.close();
			System.out.println("\nKsiążka dodana !\n");
		} catch (SQLException e)
		{
			System.err.println("Błąd przy wstawianiu książki !\n");
		}
	}

	public void dodajWypozyczenia()
	{

	}

	public void wypozyczajacy()
	{
		List<Integer> id = new LinkedList<>();
		Boolean czySaCzytelnicy = false;
		do
		{
			try
			{
				ResultSet resultSetID = statement.executeQuery("SELECT id_czytelnika FROM czytelnicy");
				while (resultSetID.next())
				{
					id.add(resultSetID.getInt("id_czytelnika"));
				}
				resultSetID.close();
				ResultSet resultSet = statement.executeQuery("SELECT * FROM czytelnicy");

				while (resultSet.next())
				{
					System.out.print("ID = " + resultSet.getInt("id_czytelnika") + "\n" + "Imię = "
							+ resultSet.getString("imie") + "\n" + "Nazwisko = " + resultSet.getString("nazwisko")
							+ "\n" + "Login = " + resultSet.getString("login"));
					czySaCzytelnicy = true;
					System.out.println();
				}
				resultSet.close();
				if (!czySaCzytelnicy)
				{
					System.out.println("Brak czytelników!");
					break;
				} else
				{
					System.out.print("Znajdz czytelnika: ");
					Integer liczba = scanner.nextInt();
					if (id.contains(liczba) == false)
					{
						System.out.println("Czytelnik o ID " + liczba + " nie znajduje się na liście czytelników!");
						continue;
					}
					doWypozyczenia(liczba);
				}
				id.clear();
				break;
			} catch (SQLException e)
			{
				System.err.println("\nProblem z wyświetleniem listy czytelników!\n");
			}
		} while (true);
	}

	public void doWypozyczenia(int idczytelnika)
	{
		List<Integer> id = new LinkedList<>();
		Boolean czySaKsiazki = false;
		do
		{
			try
			{
				ResultSet resultID = statement.executeQuery("SELECT id_ksiazki FROM ksiazki WHERE czyJest == true");
				while (resultID.next())
				{
					id.add(resultID.getInt("id_ksiazki"));
				}
				resultID.close();
				ResultSet resultset = statement
						.executeQuery("SELECT id_ksiazki, tytul, autor FROM ksiazki WHERE czyJest=TRUE");
				while (resultset.next())
				{
					System.out.print("ID = " + resultset.getInt("id_ksiazki") + "\n" + "Tytul = "
							+ resultset.getString("tytul") + " \n" + "Autor = " + resultset.getString("autor"));
					System.out.println("\n");
					czySaKsiazki = true;
					System.out.println();
				}
				resultset.close();
				if (!czySaKsiazki)
				{
					System.out.println("Brak książek !\n");
					break;
				} else
				{
					System.out.print("Która książka Cię interesuje? ");
					Integer idksiazki = scanner.nextInt();
					if (id.contains(idksiazki) == false)
					{
						System.out.println("\nKsiążka o indeksie " + idksiazki
								+ " nie znajduje się w księgozbiorze biblioteki!\n");
						continue;
					}
					wypozyczKsiazke(idczytelnika, idksiazki);
				}
				id.clear();
				break;
			} catch (SQLException e)
			{
				System.err.println("Problem z wyświetleniem danych !\n");
			}
		} while (true);
	}

	public void wyswietlListeCzytelnikow()
	{
		boolean ok = false;
		try
		{
			ResultSet resultset = statement.executeQuery("SELECT * FROM czytelnicy");
			while (resultset.next())
			{
				System.out.print("ID = " + resultset.getInt("id_czytelnika") + "\n" + "Imię = "
						+ resultset.getString("imie") + " \n" + "Nazwisko = " + resultset.getString("nazwisko") + " \n"
						+ "Login = " + resultset.getString("login"));
				System.out.println("\n");
				ok = true;
			}
			resultset.close();
			if (!ok)
			{
				System.out.println("Brak czytelników !\n");
			}
		} catch (SQLException e)
		{
			System.err.println("Problem z wyświetleniem danych !\n");
		}
	}

	public void księgozbiór()
	{
		Boolean czySaKsiazki = false;
		try
		{
			ResultSet resultSet = statement
					.executeQuery("SELECT id_ksiazki, tytul, autor FROM ksiazki WHERE czyJest = TRUE");
			while (resultSet.next())
			{
				System.out.println("ID = " + resultSet.getInt("id_ksiazki") + "\n" + "Tytuł = "
						+ resultSet.getString("tytul") + "\n" + "Autor = " + resultSet.getString("autor"));
				czySaKsiazki = true;
				System.out.println();
			}
			resultSet.close();
			if (!czySaKsiazki)
			{
				System.out.println("Brak książek w księgozbiorze biblioteki !\n");
			}
		} catch (SQLException e)
		{
			System.err.println("Wystąpił problem z wyświetleniem listy książek !\n");

			System.err.println(e);
		}
	}

	public void wypozyczKsiazke(int idczytelnika, int idksiazki)
	{
		try
		{
			PreparedStatement preparedStatement = connection
					.prepareStatement("UPDATE ksiazki set czyJest = ? where id_ksiazki = ?");
			preparedStatement.setBoolean(1, false);
			preparedStatement.setInt(2, idksiazki);
			preparedStatement.execute();
			preparedStatement.close();
			System.out.println("\nMiłego czytania !\n");
		} catch (SQLException e)
		{
			System.err.println("Wystąpił problem z operacją wypożyczenia !\n");
			System.err.println(e);
		}

	}

	public void wyswiettlListeWypozyczonychKsiazek()
	{
		List<Integer> id = new LinkedList<>();
		Boolean ok = false;
		do
		{
			try
			{
				ResultSet resultID = statement.executeQuery("SELECT id_ksiazki FROM ksiazki WHERE czyJest = false");
				while (resultID.next())
				{
					id.add(resultID.getInt("id_ksiazki"));
				}
				resultID.close();
				ResultSet resultSet = statement
						.executeQuery("SELECT id_ksiazki, tytul, autor FROM ksiazki WHERE czyJest = false");
				while (resultSet.next())
				{
					System.out.println(resultSet.getInt("id_ksiazki") + " " + resultSet.getString("tytul") + " - "
							+ resultSet.getString("autor"));
					ok = true;
				}
				System.out.println();
				resultSet.close();
				if (!ok)
				{
					System.out.println("Żadna książka nie została jeszcze wypożyczona !\n");
					break;
				} else
				{
					System.out.print("Jaka książka Cię interesuje? ");
					Integer liczba = scanner.nextInt();
					if (id.contains(liczba) == false)
					{
						System.out.println("\nKsiążka o indeksie " + liczba
								+ " nie znajduje się w zbiorze wypożyczonych książek !\n");
						scanner.nextLine();
						continue;
					}
					oddajKsiazke(liczba);
				}
				id.clear();
				break;
			} catch (SQLException e)
			{
				System.err.println("Błąd z wyświetleniem wypożyczonych książek!\n");
			}
		} while (true);
	}

	public void oddajKsiazke(int idksiazki)
	{
		try
		{
			PreparedStatement operation1 = connection
					.prepareStatement("UPDATE ksiazki SET czyJest = ? WHERE id_ksiazki= ?");
			operation1.setBoolean(1, true);
			operation1.setInt(2, idksiazki);
			operation1.execute();
			operation1.close();
			PreparedStatement operation2 = connection.prepareStatement("DELETE from wypozyczenia where id_ksiazki = ?");
			operation2.setInt(1, idksiazki);
			operation2.execute();
			operation2.close();
			System.out.println("\nDziękujemy za oddanie książki !\n");
		} catch (SQLException e)
		{
			System.err.println("Wystąpił problem z operacją oddawania !\n");
		}
	}

	public void usunTabele()
	{
		try
		{
			String usunCzytelnikow = "DROP TABLE czytelnicy";
			String usunKsiazki = "DROP TABLE ksiazki";
			statement.execute(usunCzytelnikow);
			statement.execute(usunKsiazki);

			statement.close();
			System.out.println("Operacja usuwania powiodła się!\n");
		} catch (SQLException e)
		{
			System.err.println("Operacja usuwania nie powiodła się!\n");
			System.err.println(e);
		}
	}

	public void zamknijPolaczenie()
	{
		try
		{
			connection.close();
			scanner.close();
		} catch (SQLException e)
		{
			System.err.println("Problem z zamknięciem połączenia !\n");
		}
	}
}

 

---

pewnie że bardzo opisowa coż bardziej opisuje zmienna jak słowo "ok" :D Nie za bardzo rozumiem o co Ci chodziło z tą klasa opakowującą??

Nie tak jest z nią to, że nie mówi totalnie nic co ta flaga oznacza, to że ta zmienna będzie mówić czy coś jest ok czy nie wiadomo już po jej typie. Może mówić o odpowiedzi, o sterowniku jdbc, o Twoim samopoczuciu podczas pisania kodu. Może mówić dosłownie o wszystkim i to jest problem z czytelnością. Pomyśl, że masz więcej takich flag. Biorąc pod uwagę to, że zadeklarowałeś ją na samym początku metody a nie blisko miejsca użycia to nie wiadomo po co ona jest. Co jeśli będzie potrzeba więcej flag? Drugą nazwiesz ok2? I połap się potem od czego te flagi są.

To co powinieneś był zrobić to nazwać ją  zgodnie z tym za co odpowiada, co opisuje. U Ciebie ona sprawdza czy resultset pusty, wiec tak ją nazwij. czyOdpowiedzPusta to bardzo dobra nazwa. PS teraz zauważyłem, że niżej zrobiłeś flage czySaKsiazki - tez spoko.

zrobiłem teraz dwie metody księgozbiór() teraz wygląda jak powinien a doWypozyczenia() tak jak wczesniej ten "uniwersalny" ksiegozbior

Sorry, ale mi to się dalej nie podoba.

wyswiettlListeWypozyczonychKsiazek()

tak na prawde opdpowiada za oddawanie ksiazek? Nie tego spodziewam się gdy widzę tą nazwę, spodziewam się, że wyświetli listę wypożyczonych książek i tyle. Problem tkwi w tym, że zrobiłeś za dużą metodę. Dlatego masz problem z jej nazwaniem, bo nie robi ona jednej rzeczy, tylko wiele - kłania się pierwsza zasada SOLID - single responsibility. Metody mogą i czasem bardzo powinny mieć nawet tylko kilka linijek i służy to temu, żeby ktoś czytając kod czytał wywołanie metody czyli opis tego co robi kod ukryty pod tą metodą, a nie musiał sam analizować co ten kod robi.

Skoro masz metodę wyświetlającą czytelników to po co do metody wypozyczający (której nazwa też jest straszna) właściwie przekleiłeś zawartość tej metody?

 

                    System.out.println("\n");
                    czySaKsiazki = true;
                    System.out.println();

To też mnie nieźle rozwaliło :D

                id.clear();
                break;

To nie ma sensu. Po co czyścisz listę skoro zaraz przerwiesz pętle i wyjdziesz z pętli?

A co do samej pętli to problem z nią jest taki, że pętla do while ma swoje zastosowanie tak samo jak zwykła pętla while. Gdy widzę do while to myśle, żę ktoś ją napisał daltego, że pętla ma się wykonać co najmniej raz - tzn warunek przerwania petli będzie wiadomy dopiero po pierwszym przejsciu pętli. U Ciebie tak nie jest. Jest do while totalnie bez powodu. A konstrukcja do while(true) już totalnie nie ma racji bytu bo tak jak wspomniałem wprowadza niepotrzebny zamęt.

Cała zabawa z break i continue jest zła, bo cieżko się analizuje flow tego programu. Musisz przeczytać cała zawartość żeby wiedzieć kiedy ta pętla się przerywa i w którym miejscu i dlaczego. Prześledzmy co tam się dzieje. Przerywasz pętle gdy albo uda się wyporzyczyć ksiażke lub nie ma żadnych książek. Zrób więc pętle do while(!czyUdaloSieWypozyczyc), a księgozbiór pusty możesz zostawić tak jak jest na break, bo to częsta konstrukcja, gdy coś jest nie tak - przerywamy pętle.

Nie potrafie wlasnie zrobic relacji :/ teraz czytam cos na temat JOIN (może mi to pomoze :D)

To ciekawe, bo własnie je zrobiłeś. foregin key i references, w ten sposób tworzysz relacje. Join jest całkiem od czego innego - masz dwa zapytania i chcesz je połączyć jakimś warunkiem w stylu if pierwszeZapytanie.id == drugieZapytanie.id. Wtedy połączy Ci rekordy o tej samej wartości id z dwóch różnych zapytań.

Co do nazw jeszcze np

public void doWypozyczenia(int idczytelnika)

Nazwa tak średnio. Ja bym to nazwał wypozyczKsiazke. A metodę, która teraz tak się nazywa zrobić private, bo osoba korzystająca z tej klasy nie powinna mieć dostępu do wywołania wypozyczKsiazke(9999,5943859834958395).

Answer 2

Wydzielasz część wspólną do private i inicjujesz w liście i w osobnym obiekcie?

Comments

Eeeem nie za bardzo rozumiem o co Ci chodzi :P co robie?

---

@ShiroUmizake, ja mam teraz coś takiego (wydzielenie jakkolwiek miałoby wyglądać nie wiem jak mam to zrobić i raczej mało możliwe w tym przypadku):

public void wypozyczajacy()
	{
		List<Integer> id = new LinkedList<>();
		Boolean czySaCzytelnicy = false;
		do
		{
			try
			{
				ResultSet resultSetID = statement.executeQuery("SELECT id_czytelnika FROM czytelnicy");
				while (resultSetID.next())
				{
					id.add(resultSetID.getInt("id_czytelnika"));
				}
				resultSetID.close();
				ResultSet resultSet = statement.executeQuery("SELECT * FROM czytelnicy");

				while (resultSet.next())
				{
					System.out.print("ID = " + resultSet.getInt("id_czytelnika") + "\n" + "Imię = "
							+ resultSet.getString("imie") + "\n" + "Nazwisko = " + resultSet.getString("nazwisko")
							+ "\n" + "Login = " + resultSet.getString("login"));
					czySaCzytelnicy = true;
					System.out.println();
				}
				resultSet.close();
				if (!czySaCzytelnicy)
				{
					System.out.println("Brak czytelników!");
					break;
				} else
				{
					System.out.print("Znajdz czytelnika: ");
					Integer liczba = ksiazki.nextInt();
					if (id.contains(liczba) == false)
					{
						System.out.println("Czytelnik o ID " + liczba + " nie znajduje się na liście czytelników!");
						continue;
					}
					doWypozyczenia(liczba);
				}
				id.clear();
				break;
			} catch (SQLException e)
			{
				System.err.println("\nProblem z wyświetleniem listy czytelników!\n");
			}
		} while (true);
	}

public void listaCzytelnikow()
	{
		boolean ok = false;
		try
		{
			ResultSet resultset = statement.executeQuery("SELECT * FROM czytelnicy");
			while (resultset.next())
			{
				System.out.print("ID = " + resultset.getInt("id_czytelnika") + "\n" + "Imię = "
						+ resultset.getString("imie") + " \n" + "Nazwisko = " + resultset.getString("nazwisko") + " \n"
						+ "Login = " + resultset.getString("login"));
				System.out.println("\n");
				ok = true;
			}
			resultset.close();
			if (!ok)
			{
				System.out.println("Brak czytelników !\n");
			}
		} catch (SQLException e)
		{
			System.err.println("Problem z wyświetleniem danych !\n");
		}
	}