Keskustelu: Ohjelmointikysymykset: C++ koodin parantelua (Laskin)

Avullanne (kiitos vaan) pääsin vähän alkuun c++ koodauksen kanssa.

Ensimmäinen tekemäni jotain järkevää tekevä ohjelmani on alapuolella. Koodi toimii, mutta olisinkin kysynyt palautetta koodista.

Onko jotakin jonka voisi tehdä paremmin, tai...
Kaikenlaista palautetta otetaan vastaan.

Koodissa ei ole vielä mitään tarkistuksia syötteen tarkistukseen, mutta enköhän sellaisenkin saa siihen rakenneltua.

Ohjelma kysyy ensin laskutehtävää joka suoritetaan ja sen jälkeen kaksi lukua, jonka jälkeen laskee tuloksen ja tulostaa vastauksen.

#include <iostream>

using namespace std;
int main() {

int mika;
int luku1;
int luku2;
int luku3;
cout << endl << endl << endl << endl << endl;

cout << "1 Pluslasku" << endl;
cout << "2 Miinuslasku" << endl;
cout << "3 Kertolasku" << endl;

cout << "Valitse haluamasi laskutoimituksen numero: ";
cin >> mika;


 cout << "Anna ensimmainen luku: ";
  cin >> luku1;

 cout << "Anna toinen luku: ";
  cin >> luku2;




         if (mika == 1)
         {
         luku3 = luku1 + luku2;
         }

         if (mika == 2)
         {
         luku3 = luku1 - luku2;
         }

         if (mika == 3)
         {
         luku3 = luku1 * luku2;
         }






cout << endl << endl << endl << endl  << "Yhteensa tulos on " << luku3 << endl << endl << endl << endl;
return 0;
}

1. Hassut sisennykset
2. using namespace std; sotkee nimiavaruuden. Eikö tässä using std::cout olisi parempi?
3. Kannattaa tarkistaa puskuriylivuodot.
4. If-hässäkkä houtuisi tyyllikkäämmin joko switchillä tai näin:

if (mika == 1) luku3 = luku1 + luku2;
else if (mika == 2) luku3 = luku1 - luku2;
else if (mika == 3) luku3 = luku1 * luku2;

Nämäpä tuli heti alkuun mieleen. Tätähän voisi parannella siten, että lauseke syötetään ihan tyyliin 1*2 ja sitten parseroidaan jne. Tosin se vaatii jo melkoisen määrän koodia. Samalla voisit tehdä kätevän ominaisuuden, eli lausekkeen syöttämisen parametrinä.

if (mika == 1) luku3 = luku1 + luku2;
else if (mika == 2) luku3 = luku1 - luku2;
else if (mika == 3) luku3 = luku1 * luku2;

Joo ja tuohon sais varmaan vielä tarkistuksen, perään vielä else if tulostaa virheelliset syötteet ja lopetetaan suoritus.

Tai vois olla parempi jos tarkistus suoritettaisiin heti syötteen jälkeen.

Edellä tulikin jo koodin ulkomuodosta ja if-rakenteen parannuksesta. Lisäksi kehottaisin kiinnittämään huomiota järkeviin muuttujien nimiin. Jos koodi on vähänkin pidempää tai jos tutkit vanhaa koodia, helpottaa se kummasti koodin ymmärtämistä. Kannattaa myös harkita vakavasti opettelemaan käyttämään notaatoita muuttujien kanssa. (ks. mallia vaikka http://web.umr.edu/~cpp/common/hungarian.html)

Esimerkkinä oma koodisi:
mika -> iLaskutoimitus
luku1, luku2 -> iLuku1, iLuku2
luku3 -> iTulos

Itse teen yleensä if-lauseet toisinpäin tyyliin

if (1 == iTyyppi) ....

Tällöin ei ole pelkoa siitä että erehtyy sijoittamaan lauseessa muuttujaan jotain vertauksen sijaan. (jos sattuu jäämään kääntäjän varoitukset katsomatta..)

Tuo unkarilainen koodaustapa (vai mikä nyt onkaan) ei ole mikään absoluuttinen totuus, itse en näe syytä sijoittaa tavallisten muuttujanimien eteen i/f/d:tä kertomaan tyypistä (osoittimet ja luokat ovat sitten eri asia). Toki tätä jonkin verran käytetään ja on hyvä osata lukea sitä.

Yksi mahdollisuus on kokeilla tuota tapaa käytännössä, voihan siihen vaikka tykästyä. Itse en pitänyt siitä.

En minäkään. Yleensä sitä suunnitteluvaiheessa tietää aika hyvin, mitä muuttujaan pitää voida laittaa, ja perustyyppejä ei olekaan kovin monta. Structit taas ovat siitä selkeitä, että ei ole kovin monta vaihtoehtoa, kuinka esimerkiksi koordinaatit voi esittää, eli harvemmin sattuu sekaannuksia, jos muuttujan nimi on vain "Sijainti". On muuten helpompi kirjoittaakin, kun ei tarvitse miettiä ylimääräisiä kirjaimia.

Lisäksi minusta on aika vaikea onnistua vahingossa jättämään =-merkki pois huomaamattaan, varsinkin, jos edes kerran lukaisee kirjoittamansa koodin läpi. Tuskin täällä kovin moni ruutuun katsomatta kirjoittaa hirmu pitkiä koodeja. (Kääntäjähän ei muuten varoita noista vahingollisista sijoituslauseista, koska ne ovat aivan laillisia sinänsä.)

Muuttujan järkevä nimeäminen on kuitenkin aivan ehdoton asia. Olen muutamaa vanhaa ohjelmaani tutkaillut, ja on hieman miettimistä kymmenien koodirivien pituisissa funktioissa, joiden muuttujat ovat loogisesti a, b, c, d jne. Ei siitä ota örkkikään selvää.

Ei se totuus olekkaan, lähinnä tyyliseikka. Auttaa muuten yllättävänkin paljon jos ja kun joutuu katselemaan muiden tekemiä koodeja. Ei tarvitse rullailla aina ylös muistelemaan mikähän tämäkin muuttuja taas oli tyypiltään kun sen näkee samantien nimestä. Ja sitten kun sitä on vähän aikaa käyttänyt, tulee nuo 'ylimääräiset kirjaimet' mukaan ihan automaattisesti.

Ja vielä, jotkut kääntäjät todellakin ilmoittavat if-lauseen sisällä olevasta sijoituksesta. Esim. Borland C++ Builder 6.0, default asetuksin:

[C++ Warning] Unit1.cpp(21): W8060 Possibly incorrect assignment

Visual C++ (ainakin 6.0, en tiedä mistä lähtien ominaisuus on tuotteessa ollut) näyttää muuttujan tietotyypin tooltippinä kun pitää hetken hiiren kursoria muuttujan nimen päällä. Ei tarvitse unkarilaista notaatiota. Ja kyllä koodari omat muuttujansa muistaa ;)

Unkarinotaatiosta kannattaa kuitenkin muistaa, että se tapa, miten sitä yleensä käytetään, eli ns. "Systems Hungarian", on aika turha:

http://blogs.msdn.com/larryosterman/archive/2004/06/22/162629.aspx
http://www.joelonsoftware.com/articles/Wrong.html ("I'm Hungary" -kohta)

Eli lyhyesti, pointtina ei ollut nimetä esimerkiksi kaikki kokonaisluvut nimin "iJotain", jossa "i" tarkoittaa "integer", vaan esimerkiksi:

uint xWindow, // x-koordinaatti
     yWindow, // y-koordinaatti
    cbLength; // "count of bytes" eli esimerkiksi stringin pituus

Tätä kutsutaan "Apps Hungarian":ksi. Pointtina on, että tietää, että tekee jotain väärin, jos koodissa prefiksit eivät natsaa. Jokin yWindow = cbLength ei voi olla oikein, koska miksi ihmeessä y-koordinaatiksi asetettaisiin jonkin kirjainjonon pituus?

Kaiken maailman a_lpszFoot (function argument, a long pointer to a string terminated by a zero) ja m_prhwndBarit (member of a class, a pointer to an array of handles to windows) ovat käytännössä hyödyttömiä sekä sekavia.

Niin, tämä Deewiantin esittelemä tapa on huomattavasti järkevämpi esimerkki. Perustelutkin ovat loistavat.

Mikään ei silti estä nimeämästä muuttujiansa aivan selkeällä suomella: cbLength = Tavujen_Maara. Nykyään ei ole tallennustilasta puutetta, joten muutama merkki lisää ei ole kuolemaksi. Pääasiahan on kuitenkin koodin ymmärrettävyys ohjelman kehittäjälle, eli jos on tarkoitus tehdä työt itse, niin sillä silmällä voi myös tyylinsä valita.

Itse olen huomannut, että kun globaalien muuttujien eteen laittaa sen prefiksin 'g_', lukeminen helpottuu huomattavasti. Tosin globaaleja muuttujia ei kannattaisi änkeä niin mahdottomasti ohjelmaan.