🚀 go-pugleaf

Jo Vermeulen wrote:
> 
> Sjoerd Schreuder wrote:
> > Overigens is het slordig om m_pDna, m_pC, m_pJ en m_pV niet te
> > initialiseren in de constructor.
> 
> Die initialiseer ik in de derived classes van interface, maar je hebt
> gelijk, dat kan ik evengoed hier doen.

Des te minder je overlaat aan derived classes, des te minder er fout kan
gaan. Verder voorkom je je daarmee dat dezelfde code op diverse plaatsen
komt te staan.

Mijn richtlijn is om afgeleide klassen zo min mogelijk vertrouwen.
Eigenlijk vertrouw ik geen enkele code buiten de huidige klasse.
Dan spoor je bugs veel sneller op omdat de hoeveelheid code die b.v. bij
m_pDna kan veel beperkter is. I.p.v. een protected m_useStandardInput zou
ik dan ook een function "bool UseStandardInput() { return m_useStandardInput; }"
maken en m_useStandardInput private maken. Dan weet je zeker dat niemand
die waarde kan veranderen buiten jou om.

> > Ook het gebruik van "using namespace std;" in een header is niet zo
> > handig.
>
> Ik gebruikte eerst std::cout en std::vector enz., maar Visual
> Studio versie 6 kan daar niet mee overweg. Daarom dat we dus
> genoodzaakt waren om using namespace std; te gebruiken.

Vreemd. Ik gebruik ook Visual Studio 6.0 en ik heb nog nooit
last gehad met std::cout en std::vector. Ik heb wel een recente service
pack geinstalleerd; misschien ligt het daaraan?
 
> > Verder lijkt het erop dat een File en een Dictionary steeds met
> > elkaar verband houden. Waarom heb je geen aparte klasse voor die
> > combinatie?
> 
> Hoe bedoel je? We lezen de dictionary's idd in van een file ja (wordt
> meegegeven aan de constructor).

Ik bedoel dit:
 string m_cFile;
 string m_jFile;
 string m_vFile;
 Dictionary *m_pC;
 Dictionary *m_pJ;
 Dictionary *m_pV;

Het lijkt er erg op dat m_cFile en m_pC bij elkaar horen,
netzoals m_jFile en m_pJ en m_vFile en m_pV.

Ik zou het omzetten in:
 string_and_Dictionary m_C;
 string_and_Dictionary m_J;
 string_and_Dictionary m_V;
alhoewel ik een betere naam dan "string_and_Dictionary" zou zoeken.

Nog een opmerking: waarom die "useStandardInput" variabele?
En daarnaast heb je ook een m_inputFile; mij lijkt het dat
altijd slechts een van de twee wordt gebruikt. Geef gewoon een stream mee,
dan kun je bij de aanroep beslissen of je std::cin of std::fstream gebruikt.

Verder zou ik de klasse splitsen. Een interface klasse met alleen
de puur virtuele functies, en een afgeleide klasse die alle
protected spul bevat. De interface naar buiten en de interface naar
de afgeleide klassen zijn daarmee netter gescheiden. En misschien wil
jij (of iemand anders) ooit wel een afgeleide klasse maken die wel de
interface implementeerd, maar niets heeft aan al dat protected spul.

Eigenlijk komt het erop neer dat een klasse slechts een ding moet doen.
Doordat je je op een ding concentreert, kun je dat ene ding veel beter
doen. Deze klasse doet een tiental dingen. Bovenstaande opmerkingen zijn
allemaal suggesties om een deel van die functionaliteit in een aparte klasse
onder te brengen. Het lijkt in eerste instantie overkill, maar je code
wordt er in het algemeen netter en beter begrijpbaar van. Mits je de juiste
opsplitsing kiest, en daar komt wat ervaring bij kijken. Maar dat is dan
ook een van de kenmerken van een goede OO programmeur: de juiste opsplitsing
in klassen kiezen.

Sjoerd Schreuder