Pratiche di codice Cleaner per la programmazione Java

La decisione nella mia mente si basa sul fatto che questo programma diventerà più grande o rimarrà così com'è. In ciò che mostri, sono assolutamente d'accordo sul fatto che un programma di queste dimensioni non meriti la complessità di più di una classe o di una vera implementazione orientata agli oggetti.

Tuttavia, se il tuo programma cresce bene oltre 50 linee di codice, ti manchi di vapore usando l'approccio monolitico, a causa del debito tecnico, in particolare in questo caso, a causa della mancanza di architettura modulare, la mancanza di separazione di preoccupazioni, confusione inappropriata delle responsabilità. Quindi, tra i due, il secondo stile è migliore.

Tuttavia, devi continuare ad andare avanti, perché non va abbastanza lontano.

Esiste solo una classe e troppi metodi essenzialmente non correlati alla funzione si trovano nella stessa classe. I metodi in questa classe sono tutti correlati a un particolare caso d'uso, piuttosto che essere separati in singole responsabilità.

Per uno, il principale dovrebbe probabilmente essere nella sua classe Main .

In generale, un'operazione di trasferimento dei dati dovrebbe operare su flussi già aperti (file) piuttosto che aprire il file, quindi trasferire e quindi chiudere il file, perché il modo in cui si basa sta combinando troppe responsabilità insieme. (come @PeterGelderbloem indica anche nel commento).

La chiave e i metodi di codifica dovrebbero essere incapsulati nella loro stessa astrazione in modo tale che altri approcci possano essere usati da un'applicazione (possibilmente allo stesso tempo) istanziando l'oggetto giusto e senza dover cambiare nessuno degli altri (es. trasferimento dati) ) codice.

Metodi di utilità come hex2byte non sono molto astratti e probabilmente userò solo DatatypeConverter.parseHexBinary direttamente piuttosto che costruire un'astrazione migliore per questo uso una tantum.

(Una migliore astrazione raggrupperebbe / accoppierà due conversioni insieme (avanti e indietro) in modo che l'utente possa essere sicuro della conversione da stringa e stringa usando approcci compatibili e l'intera astrazione potrebbe essere implementata usando una coppia compatibile differente senza doversi preoccupare del resto del codice usando l'altro membro della coppia corretto dal momento che l'astrazione li raggruppa insieme.)

Questo dovrebbe probabilmente andare alla revisione del codice, ma qui va:

Per iniziare, dovresti davvero ripulire l'opzione 1. L'uso di try () con la sintassi elimina gran parte dello standard di chiusura delle operazioni e il controllo dei valori null, ecc. Quindi alcune altre cose minori che potresti o non potrebbero piacerti ma arriviamo a un set di codice molto più piccolo:

import static javax.xml.bind.DatatypeConverter.parseHexBinary;

import java.io.FileInputStream;
import java.io.IOException;
import java.math.BigInteger;
import java.net.Socket;
import java.security.InvalidKeyException;
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;

import javax.crypto.Cipher;
import javax.crypto.CipherOutputStream;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.spec.SecretKeySpec;

public class FileSender {

    public static final String KEY = "00000000000000000000000000000000";
    public static final String ALGORITHM = "AES";
    public static final String FILE = "C:/Users/Andres/Desktop/file.txt";
    public static final String HASH_ALG = "MD5";

    public static void main(String [] args) throws Exception {
        System.out.println("Running client ...");
        MessageDigest md = MessageDigest.getInstance(HASH_ALG);

        byte[] byteArray = new byte[1024];

        try (FileInputStream fis = new FileInputStream(FILE)) {
            for (int bytesCount = 0; (bytesCount = fis.read(byteArray)) != -1;) {
                md.update(byteArray, 0, bytesCount);
            }
        }

        String checksum = String.format("%032x",new BigInteger(1, md.digest()));

        //Create "secret" key and inits the cipher.
        SecretKeySpec keySpec = new SecretKeySpec(parseHexBinary(KEY), ALGORITHM);
        Cipher cipher = Cipher.getInstance(ALGORITHM + "/ECB/PKCS5PADDING");
        cipher.init(Cipher.ENCRYPT_MODE, keySpec);

        // send the file
        try (Socket client = new Socket("localhost", 8081);
             CipherOutputStream cipherOut = new CipherOutputStream(
                 client.getOutputStream(), 
                 cipher);
             FileInputStream fileReader = new FileInputStream(FILE);
        ) {
            byte[] fileBuffer = new byte[1024];
            for (int len; (len = fileReader.read(fileBuffer)) != -1;) {
                cipherOut.write(fileBuffer, 0, len);
            }
        }
        System.out.println("Ended client .... ");
    }
}

Ora che abbiamo questo, è facile vedere che ci sono due cose da fare qui: creare una somma di controllo e inviare il file.

    public static String createCheckSum() throws NoSuchAlgorithmException, IOException {
      MessageDigest md = MessageDigest.getInstance(HASH_ALG);

      byte[] byteArray = new byte[1024];

      try (FileInputStream fis = new FileInputStream(FILE)) {
          for (int bytesCount = 0; (bytesCount = fis.read(byteArray)) != -1;) {
              md.update(byteArray, 0, bytesCount);
          }
      }

      return String.format("%032x",new BigInteger(1, md.digest()));
    }

    public static void sendFile() 
        throws NoSuchPaddingException, 
               InvalidKeyException, 
               NoSuchAlgorithmException, 
               IOException {

        SecretKeySpec keySpec = new SecretKeySpec(parseHexBinary(KEY), ALGORITHM);
        Cipher cipher = Cipher.getInstance(ALGORITHM + "/ECB/PKCS5PADDING");
        cipher.init(Cipher.ENCRYPT_MODE, keySpec);

        try (Socket client = new Socket("localhost", 8081);
             CipherOutputStream cipherOut = new CipherOutputStream(
                 client.getOutputStream(), 
                 cipher);
             FileInputStream fileReader = new FileInputStream(FILE);
        ) {
            byte[] fileBuffer = new byte[1024];
            for (int len; (len = fileReader.read(fileBuffer)) != -1;) {
                cipherOut.write(fileBuffer, 0, len);
        }
    }
}

Quindi il tuo principale chiama semplicemente i due metodi. Se questo è tutto ciò che devi fare adesso, allora penso che tu abbia finito (a parte la somma del checksum). Se hai bisogno di riutilizzarlo, il prossimo passo sarebbe quello di ridefinire le costanti e renderle parametri.

Per costruire un buon codice OO in Java, è necessario essere bravi a scrivere buoni metodi riutilizzabili. Tutte le funzionalità di OO di Java ruotano attorno ai metodi e cercano di utilizzare il polymporphism con metodi che non sono ben strutturati e non ti porteranno molto lontano.

the comments say mainly what is going on in a general level.

Bene, ecco il tuo problema.

È bello avere commenti quando sono necessari. Ma non è bello aver bisogno di commenti.

Ciò che ci hai mostrato nell'opzione 1 è il codice procedurale. Dici che è più facile da leggere. Bene, è semplice, piatto e flessibile come un esattore delle tasse. Ma leggibile? Bene, ho letto FileSender . Quindi, mentre i miei occhi si illuminano, mi aspetto che invii dei file. Ma mi perdo rapidamente nei dettagli. Dettagli Non sono pronto a pensare. Ora posso alimentarlo. O posso contare sui commenti. Ma farlo non è un segno di codice di facile lettura. Quindi penso davvero che sia più facile scrivere. Non è più facile da leggere.

Tieni premuto l'opzione 2 come esempio del contatore. Beh, sì, questo è difficile da leggere. Ma non è l'uso dell'astrazione che trovo distraente qui. È il rumore in più.

• Perché i loro commenti dicono la stessa cosa dei nomi dei metodi?

• Perché devo scorrere su e giù per seguirlo? O metti le definizioni dei metodi più astratte in alto o in basso. Quando sono allo stesso livello, definirli in ordine di apparizione. Non mescolarli.

• Perché stai rilevando eccezioni solo per ri-lanciarle quando non le prendi mai più? Perché sono tutti RuntimeException? Ti sta facendo bene? Perché è abbastanza distraente e non ha nulla a che fare con il fatto di essere OOP o procedurale.

• Perché restituisci cose che devono essere chiuse? Cerca il foro nel modello centrale . OO può essere funzionale.

In breve l'opzione 2 è difficile da leggere perché non è stata scritta bene. Ora ammetto che scrivere codice OO di bell'aspetto può essere difficile. Soprattutto quando non ci si abitua. Tuttavia, leggere OO buono non è così difficile.

La vera domanda allora è che ora sai cosa vuoi fare con una buona OO da cui preferiresti lavorare? Opzione 1 o Opzione 2? Beh, ammetto che l'opzione 2 non sarebbe la mia scelta. Questo significa che OO è sbagliato e procedurale è più pulito? No. Significa che OO è più espressivo. Quella più espressiva è la più facile da incasinare. È anche il potere. Usa quel potere con saggezza.

Dipende da due fattori:

1. Dimensione del programma
2. Che si tratti di un intero programma o di una parte di un programma più grande.

Quando sarebbe l'opzione 1 essere migliore?

Per un programma di quelle dimensioni o che non farà parte di un progetto più grande, potrei dire che l'opzione 1 è OK. Avere un metodo principale significa che quello è il punto di ingresso e basta. Probabilmente è un intero programma in sé.

Quando sarebbe l'opzione 2 essere migliore?

Tuttavia, se questo codice fa parte di una cosa più grande , la seconda opzione è migliore:

1. Il costruttore ti dice cosa deve fare il suo lavoro
2. L'unico metodo pubblico è tutto ciò che devi sapere per usarlo da qualsiasi luogo.

Se il codice era considerevolmente più grande o complesso , la seconda opzione sarebbe anche migliore:

1. Più facile testare le singole parti (invio del file, creazione dell'hash, ecc.)
2. È più facile apportare correzioni che non creino nuovi bug in altre parti del programma a causa di ambiti di metodi più piccoli
3. Con un approccio più modulare non è necessario leggere l'intero programma per comprenderlo. Quindi hai meno da leggere e meno da pensare allo stesso tempo.