Utilizzo della classe privata nidificata per conservare i dati dal file csv

0

Ho una classe con un solo metodo. Questo metodo deve leggere il file csv, fare un po 'di lavoro sulla logica interna (controllare se l'elemento esiste già, fare alcune trasformazioni, ecc.) E infine scrivere tutti i dati in un database. Sto usando una classe nidificata per l'archiviazione dei dati dal file CSV come elenco di oggetti. Sembra qualcosa del genere:

public class Importer
{
    public void Import(string path)
    {
        using(var reader = File.OpenText(path))
        {
            var csv = new CsvReader(reader);
            var items = csv.GetRecords<ItemCsvModel>().ToList();
            //do some work and insert items to db
        }
    }

    private class ItemCsvModel
    {
        public int Id {get; set;}   
        public string Name {get; set;}
        public string Description {get; set;}
    }
}

Il recensore del codice non ha gradito l'uso di classi nidificate mi ha detto che sarebbe stato meglio se questa classe venisse spostata su un altro file. Penso che questa classe sia solo un dettaglio di implementazione interno e non ha motivo di essere visibile al di fuori della classe Importer. Potrei leggere il file csv senza usare alcuna classe semplicemente facendo qualcosa del genere:

var id = csv.GetField<int>("Id");
var name = csv.GetField<string>("Name");
var description = csv.GetField<string>("Description");

Ma ho scelto un altro approccio. Al momento non ci sono altri posti dove vengono letti i file csv e dubito che cambierà molto presto, quindi non sto ripetendo più volte dichiarazioni di classe. Quali sono i tuoi pensieri?

    
posta user1242967 06.04.2016 - 18:22
fonte

3 risposte

1

Non vedo alcun problema con esso.

Forse in fondo alla strada se si vuole implementare un diverso tipo di parser (xml, diciamo), ma in tal caso si estrae l'implementazione csv esistente, dargli un'interfaccia comune e iniettarla (o il parser xml) come un IParser o altro.

Questa è una delle ragioni per cui sarebbe logico spostarla, ma sono anche fermamente convinto che non si debba sovra-progettare per il codice che a) non è attualmente necessario, e b) può essere facilmente refactato se è necessario.

Per me, questo soddisfa sia i criteri aeb che sopra. Non seguo davvero molto XP, ma la mentalità YAGNI è una delle migliori pepite di saggezza che ho acquisito in OOP (YAGNI: Non ne avresti bisogno).

(una volta che ho letto un grande articolo su questo tipo di over-engineering nel contesto di Java ... l'autore ha iniziato con una classe e un metodo semplici che avrebbero funzionato bene, ottenuto poi il lavoro, poi sono passati attraverso un processo di pensiero "tipico" su come tutto doveva essere "supposto", e alla fine la classe era un tale caos che non si poteva nemmeno dire cosa stava facendo ... mi piacerebbe se qualcuno avesse il link, mi sembrava di l'ho perso)

    
risposta data 06.04.2016 - 19:36
fonte
0

Direi che la tua classe sta facendo troppe cose. Sono le operazioni sui file, l'analisi CSV, i calcoli e l'archiviazione del database. Questo è il vero problema. Tutto questo viola il Principio di singola responsabilità (S in SOLID) . Se dividi questo in almeno quattro classi, potresti vedere una soluzione migliore. L'altro commentatore fa un buon punto su cosa succederebbe se il file dovesse cambiare da CSV a XML o JSON o qualsiasi altra cosa. Pensaci e chiediti perché la classe che gestisce l'accesso al database dovrebbe cambiare se il tuo formato di file cambia. Non dovrebbe.

    
risposta data 06.04.2016 - 21:32
fonte
0

Ho la stessa soluzione per gli esportatori CSV. Ogni esportatore ha il proprio trasformatore (entità-oggetto []).

Transformer è una classe interna privata. La mia soluzione implementa la mia gerarchia di interfacce ICSVExporter, ICSVEntityTransformer. È il mio modo di mettere in pratica il modo in cui fare per ulteriori sviluppi (e il modo di evitare che i junior realizzino il codice bolied-plate)

L'architetto del progetto mi ha dato il suo ok. Andava bene.

Il tuo reviwer sta dando la sua opinione. Chiedigli delle sue ragioni. Scommetto che avrai una risposta per i suoi dubbi e le sue paure.

Il tuo codice è chiaro, semplice e FUNZIONALE.

Pensare molto agli ulteriori aggiornamenti è un eccesso di reingegnerizzazione.

Suggerirò solo di avere Reader e Processor in componenti diversi. Chi legge può o non può essere chi elabora i dati di input.

Pensa in modo diverso per caricare i file CSV:

  • FileSystemCsvReader
  • FtpCsvReader
  • WebserviceCsvReader
  • QueueCsvReader
  • WhateverITComeFromCsvReader

Tutti svolgono lo stesso compito, ma in modi diversi. Ma il modo di analizzare i dati in oggetti Java potrebbe essere lo stesso.

Hai diversi CSV? Quindi potresti avere 1 processore e N trasformatori. Allora sì, prendi quella classe interna dal Processore.

... se fai una soluzione flessibile. Anche con le classi interne può essere fatto.

    
risposta data 06.04.2016 - 23:06
fonte

Leggi altre domande sui tag