Nybegynner - kritikk av struktur o.l.

[Imaginary]

 

Endret 23. juli 2011 av Imaginary

[endrebjo]

Hva skjer med ulikhetsoperatorene hvis du har negativ nevner på én av sidene?

[Dead_Rabbit]

Okey, har ikke sett så veldig grundig gjennom, men:

• I C++ er det vanlig at header filer slutter på .hpp (veit mange bruker .h, men sånn eeegentlig)
  
• Der du sjekker om nevneren er null, ville jeg kasta et exception (f.eks. std::logic_error). Hvis nevneren er null har brukeren åpenbart gjort noe dumt, og det er ikke rimelig å anta at brukeren egentlig mener at nevneren skal være 1.
  
• Du bruker litt vel mange parenteser for min smak. operator*/operator/ har høyere prioritet enn operator+/operator-, så i f.eks. operator+, trenger du ikke parenteser i første uttrykket. Det er heller ikke nødvendig med parentes rundt det du returnerer fra en funksjon.
  
• Underscore på medlemmer er jæ-ve-lig stygt (min mening), men, det kan være nyttig for å få et rent og fint interface på klassen. Dvs. det er en fin ting å bruke for å slippe funksjoner som getNumerator og setNumerator da du heller kan ha en funksjon som heter numerator som du overlaster (dette er penere). Men, du bruker underscore og getNumerator, og da faller det eneste argumentet for å bruke underscore bort (synes jeg ihvertfall).
  
• Det er ingen grunn til at operator==, operator!=, operator<, operator>, etc. ikke skal være medlemsfunksjoner av Rational. Unntaket er når man overlaster operator<< og operator>>.
  
• Å deklarere flere funksjoner i ett uttrykk (separert med komma, som du gjør i headerfilen din) er direkte nasty, og gjør koden mindre leselig. Jeg trodde først det var en typo fra din side fordi jeg er så vant til at deklarasjoner er i et eget uttrykk.
  

Endret 12. februar 2010 av Dead_Rabbit

[Imaginary]

 

Endret 23. juli 2011 av Imaginary

[Dead_Rabbit]

Underscore tok jeg fra en C++ style guide, men ser argumentet ditt. Egentlig synes jeg det faktisk uthever privat-medlemmene.

Jepp, og bruk av underscores er i utgangspunktet helt greit, men poenget med å gjøre det faller litt bort hvis man likevel bruker getSomething og setSomething. Forresten, det er mange støgge-støgge "style guidelines" der ute. Jeg sier ikke at den du har sett på er støgg, men bruk noe du selv er konfortabel med, og ikke ta alt av de greiene der for god fisk.

 

Stusset på dette selv, men oppgaven var formulert slik.

En liten "test" for å vise at operatorene ikke bør være globale. Hvor er det meningen at compare-funksjonen skal brukes? Jo, i operator==, operator<=, osv. Nå er compare tilgjengelig for alle, og dette bryter litt med prinsippet om objektorientering og enkapsulering. En løsning hadde vært å ha en friend-deklarasjon i Rational-klassen, og så gjort compare private, men dette er unødvendig da operatorene ikke trenger å være globale i utgangspunktet.

 

Det bidrar jo til å gruppere funksjonene? Kan dermed ikke se at det blir mindre leselig? Sikker på at dette ikke bare er en vane hos deg, og at du ut fra dette trekker konklusjoner om mindre lesbarhet?

Ja, det er jeg sikker på. Jeg kan faktisk ikke huske å ha sett den måten å deklarere funksjoner på hverken i kodeprosjekter jeg har deltatt i, eller i open source prosjekter.

Endret 12. februar 2010 av Dead_Rabbit

[Imaginary]

 

Endret 23. juli 2011 av Imaginary

[genstian]

Rational::Rational() : numerator_(0), denominator_(1)
{
;
};

 

Det kan da umulig fungere?

[Dead_Rabbit]

Det fungerer fint hvis du sikter til det "tomme" statmentet. Et "tomt" statement er et statement nevertheless.

[SjonkelSID]

[*] Det er ingen grunn til at operator==, operator!=, operator<, operator>, etc. ikke skal være medlemsfunksjoner av Rational. Unntaket er når man overlaster operator<< og operator>>.

1) Pga at compare() funksjonen er tilgjengelig for alle så trenger ikke operator<, operator== osv tilgang til klassens private medlemmer. Derfor trenger de ikke å være medlemsfunksjoner. Fordi de ikke trenger å være medlemsfunksjoner bør de heller ikke være det fordi klassen bør være en minimal, innkapslet enhet.

2) For klasser som har en ikke-explicit constructor som tar ett argument er det enda en grunn til at disse funksjonene ikke skal være medlemsfunksjoner:

struct Feil
{
 int tall;
 Feil(int n):tall(n) {}
 bool operator==(Feil const& f) const { return tall == f.tall; }
};

struct Riktig
{
 int tall;
 Riktig(int n):tall(n) {}
};

bool operator==(Riktig const& venstre, Riktig const& hoyre )
{
 return venstre.tall == hoyre.tall;
}

int main()
{
 Feil f(1);
 f == f;
 f == 2;
 2 == f;    // kompilerer ikke, kompilatoren kan ikke konvertere f til en int og så sammenligne
 Riktig r(1);
 r == r;
 r == 2;
 2 == r;    // kompilerer helt ok
}

Endret 14. februar 2010 av SjonkelSID

[Dead_Rabbit]

1) Pga at compare() funksjonen er tilgjengelig for alle så trenger ikke operator<, operator== osv tilgang til klassens private medlemmer. Derfor trenger de ikke å være medlemsfunksjoner. Fordi de ikke trenger å være medlemsfunksjoner bør de heller ikke være det fordi klassen bør være en minimal, innkapslet enhet.

2) For klasser som har en ikke-explicit constructor som tar ett argument er det enda en grunn til at disse funksjonene ikke skal være medlemsfunksjoner:

1) At compare-funksjonen er tilgjengelig for alle er jo et argument for å gjøre operatorene medlemmer av klassen. Siden trådstarter har overlasta oeprator<, etc, er det jo meningen av brukeren av klassen skal benytte disse. Derfor er compare-funksjonen i praksis en implementasjonsdetalj av klassen, og bør derfor være privat. At de ikke på død og liv trenger å være medlemsfunksjoner fordi koden kompilerer (hvis man søpler til interfacet med en implementasjonsdetalj som compare()), er etter min mening irrelevant, for det er dårlig design.

 

Forøvrig så er jeg heller ikke enig i at man bør gjøre et poeng av å ha så små klasser som mulig. At en klasse skal være en innkapslet enhet er riktig, men det betyr ikke at den skal være minimal. Like viktig er det at det er en logisk enhet, og funksjonalitet som gjelder klassen bør derfor være i klassen med mindre man har spesielle grunner til å ikke gjøre det (som man f.eks. har ved operator<< og operator>>).

 

2) Ja, jeg ser poenget ditt, og akkurat med f.eks. operator== er det et godt poeng, men generelt så bør ting være medlemmer i klassen når det gir mening.

Endret 14. februar 2010 av Dead_Rabbit

[SjonkelSID]

Jeg kikket på den opprinnelige oppgaven og her påpekes det at operator== osv skal implementeres som ikke-medlemmer og bruke "get"-funksjonene, så det gjør det etter min mening til bare enda bedre klassedesign. Og jeg mener fortsatt at klasser skal være minimale: klasser er ikke moduler eller mini-biblioteker.

[endrebjo]

Oppgaven er laget slik for at elevene skal lære seg de forskjellige måtene å overlagre operatorer på.

Hvis oppgaven skulle gjenspeile et skikkelig design skjønner jeg ikke logikken bak å implementere inkrementell- og shorthand-operatorene som medlemmer, og de vanlige aritmetiske og sammenliknings-operatorene som ikke-medlemmer/venner. De aritmetiske operatorene og shorthand-operatorene gjør nemlig nøyaktig det samme, bare med en annen syntaks. I mine øyne ville det derfor vært naturlig å implementere de på nøyaktig samme måten.

[SjonkelSID]

Oppgaven er laget slik for at elevene skal lære seg de forskjellige måtene å overlagre operatorer på.

Hvis oppgaven skulle gjenspeile et skikkelig design skjønner jeg ikke logikken bak å implementere inkrementell- og shorthand-operatorene som medlemmer, og de vanlige aritmetiske og sammenliknings-operatorene som ikke-medlemmer/venner. De aritmetiske operatorene og shorthand-operatorene gjør nemlig nøyaktig det samme, bare med en annen syntaks. I mine øyne ville det derfor vært naturlig å implementere de på nøyaktig samme måten.

 

Jeg er ikke sikker heller(*), jeg er ikke C++ guru på noen måte. Det er mulig jeg er indoktrinert pga at dette er den vanlige måten å gjøre det på. En forskjell er at disse funksjonene endrer et av argumentene de opererer på, så de er asymmetriske. Og alle unntatt postinkrement/dekrement returnerer en referanse til det første argumentet. Kanskje det spiller inn?

 

I hvert fall kan medlemsfunksjoner kalles på midlertidige variabler, mens en kan ikke kalle Type& operator+=(Type& a, Type const& b) på a dersom a er en midlertidig variabel. Om det er bra eller ikke vet jeg ikke, men konsekvensen blir som følger:

 

#include <iostream>

struct Uvanlig
{
 int tall;
 Uvanlig(int n):tall(n) {}
};

Uvanlig& operator+=(Uvanlig& venstre, Uvanlig const& hoyre)
{
 venstre.tall += hoyre.tall;
 return venstre;
}

Uvanlig hei1()
{
 return Uvanlig(1);
}

struct Vanlig
{
 int tall;
 Vanlig(int n):tall(n) {}
 Vanlig& operator+=(Vanlig const& v)
 {
   tall += v.tall;
   return *this;
 }
};

Vanlig hei2()
{
 return Vanlig(1);
}

int main()
{
 Uvanlig u(2);
 Vanlig v(2);
 std::cout << ( hei1() += u ).tall << '\n'; // kompilerer ikke, kan ikke binde temporær variabel til ikke-const referanse
 std::cout << ( hei2() += v ).tall << '\n'; // kompilerer, kan kalle ikke-const medlemsfunksjoner på temporære.  God eller dårlig kode?
}

 

(*) note: en grunn går via Sutters Guru of the Week 4 tilbake til Lakos 1996 hvor han angir potensielle feil relatert til binding av temporære variabler til ikke-const referanser, men siden disse ikke er lov faller vel det vekk.

Endret 15. februar 2010 av SjonkelSID

[ze5400]

Overlagre?

 

Er dette en korrekt oversettelse som har gått meg hus forbi, eller skal det være overlaste?

 

Egentlig burde vel oppgavene vært gitt på Engelsk?

Norsk egner seg (imho) svært dårlig til å beskrive slikt...

[Flin]

En ting jeg ikke skjønner er hvorfor du ikke bruker operatorene du har laget på forhånd når du skal lage >= osv.