Impedire a un parser di trasformarsi in un oggetto (apparentemente) di dimensioni divine

7

Quindi ho un programma il cui scopo è prendere i file di testo e analizzarli in un formato binario che un sistema integrato comprende. Tuttavia, il formato di testo che ho ereditato che devo analizzare è sufficientemente complesso che dopo aver refactoring la routine parse principale mi rimane una classe con più di 50 metodi che quasi tutti assomigliano a parseChannel , parseWCommand , parseVCommand , parsePCommand , parseLoop , parseHex , parseInt , ecc. ecc.

Inutile dire che la classe, dalla sua dichiarazione, sembra enorme e scoraggiante.

Tuttavia, i metodi per interfacciarsi con la classe sono estremamente semplici, solo parse (compila il testo, calcola le dimensioni compilate) e link (correggi i puntatori una volta che la posizione in memoria è nota, restituisci la finalizzazione dati binari non elaborati). In realtà, per l'utente la classe non ha praticamente nessun'altra ragione per l'esistenza oltre a quelle due funzioni, ma nella dichiarazione di classe c'è così tanta roba apparentemente inutile che è difficile persino vedere che cosa dovrebbe fare. C'è una situazione simile in corso con il numero piuttosto grande di membri dei dati che, ancora una volta, sono inutili a tutto tranne i metodi interni della classe che hanno bisogno di loro di parlarsi, anche se non so se questo è un problema.

Ho preso in considerazione l'idea di creare una classe separata esclusivamente per l'analisi utilizzata nel metodo parse , ma mi sembra strano creare una classe completamente separata che viene utilizzata solo in un singolo metodo in una singola classe. Sembra un po '... superfluo, immagino? E non so nemmeno se questo sta attaccando il problema giusto.


Credo che alla fine, ecco cosa sto chiedendo:

  1. Questa classe apparentemente enorme è effettivamente un problema? Non penso che sia rigorosamente un "oggetto dio", ma appare superficialmente come uno.
  2. Se si tratta di un problema, quali sono i metodi migliori per risolverlo?
posta Matt Sz. 19.12.2013 - 02:02
fonte

4 risposte

5

Ditch the OOP

Probabilmente non è necessario che le funzioni membro siano. Puoi fare in modo che la classe Parser fornisca alcune operazioni primitive (accetta carattere, backtrack, & c.) E implementa il resto del parser come funzioni libere che prendono un riferimento Parser . Possono vivere nel file sorgente (sotto uno spazio dei nomi anonimo) mantenendo così l'intestazione bella e minima.

In questo senso, Parser è in realtà un involucro minimo attorno a uno stato del flusso.

Inoltre, aiuta a calcolare i combinatori come many , choice e così via, per evitare di scrivere loop manuali inclini agli errori e prolissi.

Sketch

In Parse.h , la tua intera API pubblica.

unique_ptr<const Program> parse(istream&);

Lo stato del parser può essere interamente privato a Parse.cpp .

struct Parser {

  Parser(istream& stream) : stream(stream) {}

  bool accept(const char expected) {
    char actual;
    if (!stream.get(actual)) return false;
    if (expected == actual) return true;
    stream.seekg(-1, ios::cur);
    return false;
  }

  template<class F, class O>
  bool accept_if(F predicate, O output) {
    char actual;
    if (!stream.get(actual)) return false;
    if (predicate(actual)) {
      *output++ = actual;
      return true;
    }
    return false;
  }

  void expect(const char expected) { if (!accept(expected)) throw ...; }

  void push_mark() { marks.push_back(stream.tellg()); }
  void pop_mark() { stream.seekg(marks.back()); drop_mark(); }
  void drop_mark() { marks.pop_back(); }

private:
  istream& stream;
  vector<istream::pos_type> marks;
};

L'implementazione dell'API pubblica è appena in avanti per avviare la produzione di una grammatica con un nuovo parser.

unique_ptr<const Program> parse(istream& stream) {
  return parse_program(Parser(stream));
}

Gli iteratori di output sono un modo abbastanza conveniente per produrre più risultati.

template<class F, class O>
bool many_if(Parser& parser, F predicate, O output) {
  bool success;
  while (parser.accept_if(predicate, output)) success = true;
  return success;
}

I puntatori hanno il vantaggio di una conversione in bool.

unique_ptr<const Expression> number(Parser& parser) {
  string digits;
  auto append = back_inserter(digits);
  return many_if(isdigit, append) ? make_unique<Number>(digits) : nullptr;
}

E così via. I veri punti di forza di C ++ per sfruttare al meglio la scrittura dei parser sono:

  • Programmazione generica
  • Iteratori
  • Streams

Oggetti, non tanto.

    
risposta data 19.12.2013 - 02:46
fonte
3

Penso che tu sia bloccato: un grosso grammofono ha bisogno di un grande parser.

Potresti riuscire a suddividere la grammatica in "sottocramerali", ad es. una grammatica per "desrciptions di oggetti" e un'altra grammatica per "regole di azione"; ma questo è possibile solo se la tua grammatica può essere suddivisa in "stanze" separate.

    
risposta data 19.12.2013 - 02:27
fonte
2

Suppongo che il parser sia stato scritto in uno stile procedurale. In tal caso, sono previsti circa 50 metodi (= regole nella grammatica) per una lingua di piccole e medie dimensioni. Poiché non c'è modo di ridurlo, qualsiasi critica al numero di funzioni è nulle.

Tuttavia, è spesso il caso che la tua grammatica può essere vista come più subgrammmi in gran parte non collegati. Ad esempio in Markdown (il markup che i nostri post stanno utilizzando), la grammatica a livello di blocco e la grammatica in linea sono per lo più separate. Se la tua lingua ha una distinzione simile, scrivere vari parser separati che si interfacciano tra loro non solo migliorerebbe il tuo design ma anche la testabilità.

C'è anche una differenza tra parsing e lexing - alcune delle tue funzioni come parseInt non appartengono necessariamente al livello di parser (ma ciò dipende molto).

Se gli unici metodi pubblici del tuo oggetto parser sono parse e link , allora non ci sono problemi perché il resto può essere privato e implementato con qualsiasi disegno tu voglia. Il metodo link tuttavia probabilmente non appartiene a un parser e potrebbe dover essere scomposto in una classe separata - applica il Principio di responsabilità singola come ha senso.

Una nota sul termine Oggetto di Dio : questo termine è usato quando

  • una classe ha troppe responsabilità,
  • utilizza lo stato globale mutante anziché lo stato incapsulato (ovvero gli oggetti più piccoli) o gli argomenti delle funzioni e
  • è scritto in uno stile procedurale, piuttosto che orientato agli oggetti.

I primi due sono odori di codice reale che hanno per essere indirizzati: il primo è ovvio, e il secondo è assolutamente non necessario in un parser, che può essere implementato bene senza mutare nessuno stato (assumendo stai cercando di costruire un AST). Il terzo può essere una critica valida, ma a volte (ad esempio con parser), una soluzione procedurale o funzionale è più elegante di una orientata agli oggetti.

    
risposta data 19.12.2013 - 02:24
fonte
2

There's a similar situation going on with the rather massive number of data members that, again, are useless to everything but the class's internal methods that need them to talk to each other

Sospetto che sia il vero problema. Avere una classe per essere un contenitore per molte funzioni private, per un programma scritto in uno stile non OO non è un problema per se stesso (come altri hanno affermato qui) - purché si abbiano per lo più funzioni pure, senza alcuna stato o accesso ai membri dei dati. Tali funzioni possono essere ancora testate una per una, e quando si modifica il comportamento di una funzione, il rischio di interrompere altre funzioni è ancora molto piccolo.

Ma quando queste molte funzioni dipendono anche da molti membri dei dati e comunicano da loro, allora siete sulla strada del "codice spagetti", perché ora introducete delle dipendenze dove "tutto dipende da tutto" e un cambiamento in una parte del parser può provocare effetti collaterali indesiderati sulle altre parti.

Quindi il mio consiglio è: rendere le funzioni del parser più funzionali. Rendi la comunicazione più esplicita - non utilizzare "variabili globali" (le tue variabili membro sono di fatto globali dal punto di vista del parser), ma parametri di input e output espliciti. Questo probabilmente aumenterà la manutenibilità e l'evolvibilità a un livello che è ancora possibile gestirlo quando il parser ottiene sempre più metodi per tutta la durata del sistema.

    
risposta data 19.12.2013 - 09:45
fonte

Leggi altre domande sui tag