jak poprawić istniejący kod?

0

Jak powinienem zmodyfikować swoja metode, zeby wyglądało i działała poprawnie?

  public void run() {
        Connection con = getConnection();
        while (true) {
            QueueNotify actionFileContent = null;
            try {
                actionFileContent = sharedQueue.take();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            PreparedStatement ps = null;
            try {
                ps = db.writeData(con, actionFileContent);
            } catch (Exception e) {
                e.printStackTrace();
            } finally {
                db.close(con, ps);
            }
        }
    }
    private Connection getConnection() {
        Connection con = null;
        try {
            con = db.connect();
        } catch (SQLException e) {
            e.printStackTrace();
        } catch (ClassNotFoundException e) {
            e.printStackTrace();
        }
        return con;
    }
1

Po pierwsze na obrazku są 2 metody.
Po drugie - po czym poznamy, że działa lub nie poprawnie. Co to ma robić?

0

@jarekr000000:
obie metody generelnie działaja. Chodzi o to, żeby kod lepiej wyglądał i nie bylo nawiązywane połączenie z bazą za każdym razem tylko żeby np działalo dopoki nie wyłączy się program.

0
  1. Przecież connection jest wywołane raz - na początku run
  2. żeby to poprawić trzeba by:
    a ) poznać co jest w metodzie: db.writeData(con, actionFileContent);
    b) co jest db.close
    c) resztę kodu wywalić
0

zrób 1 try tam gdzie masz 2 try czyli:

zamiast

try {
  // code1
} catch (FirstException e){
  // code1e
}
try {
  // code2
} catch (SecondException e){
  // code2e
}

zrób:

try {
  // code1
  // code2
} catch (FirstException e) {
  // code1e
} catch (SecondException e) {
  // code2e
}

a wyjątek z Connection możesz wyrzucać i obsługiwać w run

0

czy takie cos ma sens, czy musze jescze zamykać połączneie z baza?

public void run() {
        Connection con = null;
        try {
            con = db.connect();
        } catch (ClassNotFoundException e) {
            e.printStackTrace();
        } catch (SQLException e) {
            e.printStackTrace();
        }

        QueueNotify actionFileContent = null;
            try {
                actionFileContent = sharedQueue.take();
            PreparedStatement ps = null;
            try {
                ps = db.writeData(con, actionFileContent);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
            } catch (Exception e) {
                e.printStackTrace();
            }
        }
0
  1. Wywal konstruktor i dodaj adnotacje @RequiredArgsConstructor. To samo z adnotacjami dla logerów. Nie trać czasu na barok Javy.
  2. W dobrej aplikacji gdy leci wyjątek to wymaga to działania człowieka. Dlatego logujemy je podając jako ostatni argument do loggera, żebyśmy zawsze mieli pełny stacktrace. Polecam talk z Geecon pt. "Actionable exceptions"
  3. Skoro napisałeś klasę, która ma metodę close, to na 99% powinna również implementować interfejs Closable.\
  4. Od Javy 1.7 mamy try-with-resources. Pozwoli Ci to wywalić bloki finally. Warunek to to, że klasa musi implementować Closeable.
  5. Podział odpowiedzialności klas leży. DatabaseService powinien albo zarządzać lifecycle Connection / PreparedStatement, albo nie dotykać. U Ciebie zamyka obiekty, nad którymi nie ma kontroli. To bez sensu.
  6. Dla purystów obsługa InterruptedException również fatalna. W Javie nie można zakończyć natychmiastowo wątku. Dlatego ten wyjątek, wskazuje Ci, że podczas pobierania z kolejki ktoś próbuje zastopować Twój wątek. I zamiast go olewać, albo wypada zakońzzyć metodę run, albo ustawić flagę Thread.interupted() szczególnie jeżeli nie Ty uruchomiłeś wątek, a jakiś Executor.

Ogólnie częsty i w miarę dobry wzorzec to mieć jedną klasę, która trzyma Connection lub ich pulę. Ogarnia kiedy są tworzone i w jakiej ilości.
I drugą klasę, która dostaje Connection, tworzy PreparedStatement, mapuje wynik na obiekt i zamyka Statement.

1

@nie100sowny: czemu stosować @NoArgsConstructor , jak domyślnie konstruktor bezparemtrowy jest definiowany automatycznie?

0

Zobacz coś w tym stylu (kodu mniej - klawiaturze lżej no i preparedStatement użyte zgodnie z przeznaczeniem)

public void run() {
        try (Connection con = db.connect()) {
            PreparedStatement preparedStatement = db.prepareStatement(con);
            while (true) {
                QueueNotify actionFileContent = sharedQueue.take();
                preparedStatement.setString(1, actionFileContent.getAction());
                preparedStatement.setString(2, actionFileContent.getName());
                preparedStatement.setString(3, actionFileContent.getContent());
                preparedStatement.executeUpdate();
            }
        } catch (SQLException  | ClassNotFoundException | InterruptedException e) {
            throw new RuntimeException(e);
        } 
    }
class DatabaseService {
    private final String jdbcDriverStr;
    private final String jdbcURL;

    public DatabaseService(String jdbcDriverStr, String jdbcURL) {
        this.jdbcDriverStr = jdbcDriverStr;
        this.jdbcURL = jdbcURL;
    }

    public Connection connect() throws ClassNotFoundException, SQLException {
        Class.forName(jdbcDriverStr);
        Connection connection = DriverManager.getConnection(jdbcURL);
        return connection;
    }

    public PreparedStatement prepareStatement(Connection connection) throws SQLException {
        return connection.prepareStatement("insert into test_table values (DEFAULT ,?,?,?)");
    }
}

Te exceptiony troche słabe w run(), ale nie bardzo wiem co to za kod i co robi, żeby coś o sensownej obsłudze napisać. Throw new RuntimeException(e) to taki default (w miarę bezpieczny).
e.printStackTrace() to proszenie się o problemy.

0

@jarekr000000: dziękuję;)

0

a @jarekr000000 po wprowadzeniu zmian w kodzie, przy tworzeniu pliku mam taki wyjątek:
java.nio.file.NoSuchFileException: /home/kowalskii/Desktop/Notify.goutputstream-LMNMEZ
at sun.nio.fs.UnixException.translateToIOException(UnixException.java:86)
at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
at sun.nio.fs.UnixFileSystemProvider.newByteChannel(UnixFileSystemProvider.java:214)
at java.nio.file.Files.newByteChannel(Files.java:361)
at java.nio.file.Files.newByteChannel(Files.java:407)
at java.nio.file.spi.FileSystemProvider.newInputStream(FileSystemProvider.java:384)
at java.nio.file.Files.newInputStream(Files.java:152)
at java.nio.file.Files.newBufferedReader(Files.java:2784)
at java.nio.file.Files.lines(Files.java:3744)
at java.nio.file.Files.lines(Files.java:3785)
at ContentReader.readContent(ContentReader.java:10)
at ScanFolderThread.create(ScanFolderThread.java:86)
at ScanFolderThread.run(ScanFolderThread.java:48)

program słuzy do sprawdzania zmian w folderze i zapisyswaniu ich w bazie danych.

0
  1. Jest wyraźnie napisane że nie ma podanego pliku.
  2. Ten błąd nie ma nic wspólnego z kodem, który poprzednio wysłałes - to jakaś inna historia.
0

@jarekr000000: ostatnie pytanie;) powinienem sobie gdzies zamknac polaczenie z baza danych?

0

Tak. W kodzie, który podałem, zamykanie było.

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