Program do obsługi liczb zespolonych – code review

0

Mam napisać program do obsługi liczb zespolonych. Jestem raczej na poziomie podstawowym, więc pewnie dodawanie, odejmowanie i moduł wystarczą, jak myślicie? :D
Ale coś mi kod nie chce się kompilować. Mógłby ktoś zerknąć i powiedzieć o co chodzi. Jeśli macie uwagi do kodu i jego chaotyczności lub są jakieś nie potrzebne elementy, to piszcie. ;)
Na razie nic nie dawałam w funkcji main, bo chyba i tak najpierw musi mi zadziałać kod. Z góry dzięki ;)

#include <iostream>
#include<cmath>

using namespace std;

class Zespolona{
    int rzeczywista;
    int zespolona;
public:
    Zespolona::Zespolona();
    Zespolona::Zespolona(int a, int b);
    Zespolona::~Zespolona;
    int Zespolona::getRzeczywista();
    Zespolona::setRzeczywista(int a);
    Zespolona operator+(const zespolona1 & const zespolona2);
    Zespolona operator-(const zespolona1 & const zespolona2);
    double Zespolona::modul(const zespolona1);

};
Zespolona::Zespolona(){
    rzeczywista = 1;
    zespolona = 1;
}
Zespolona::Zespolona(int a, int b){
    rzeczywista = a;
    zespolona = b;
}
Zespolona::~Zespolona(){
    cout<<"Koniec"<<endl;
}
int Zespolona::getRzeczywista(){
    return rzeczywista;
}
Zespolona::setRzeczywista(int a){
    rzeczywista = a;
}
Zespolona operator+(const zespolona1 & const zespolona2)
{
    Zespolona zesp1;
    zesp1.rzeczywista = zespolona1.rzeczywista+zespolona2.rzeczywista;
    zesp1.zespolona = zespolona1.zespolona + zespolona2.zespolona;
    return zesp1;
}
Zespolona operator+(const zespolona1 & const zespolona2)
{
    Zespolona zesp1;
    zesp1.rzeczywista = zespolona1.rzeczywista+zespolona2.rzeczywista;
    zesp1.zespolona = zespolona1.zespolona + zespolona2.zespolona;
    return zesp1;
}
Zespolona operator-(const zespolona1 & const zespolona2)
{
    Zespolona zesp1;
    zesp1.rzeczywista = zespolona1.rzeczywista-zespolona2.rzeczywista;
    zesp1.zespolona = zespolona1.zespolona - zespolona2.zespolona;
    return zesp1;
}
double Zespolona::modul(const zespolona1){
    mod = sqrt(zespolona1.rzeczywista^2+zespolona1.zespolona^2);
    return mod;
}
int main()
{
    return 0;
}
0
maxmiks napisał(a):

Mam napisać program do obsługi liczb zespolonych. Jestem raczej na poziomie podstawowym, więc pewnie dodawanie, odejmowanie i moduł wystarczą, jak myślicie? :D
Ale coś mi kod nie chce się uruchamiać. Mógłby ktoś zerknąć i powiedzieć o co chodzi. Jeśli macie uwagi do kodu i jego chaotyczności lub są jakieś nie potrzebne elementy, to piszcie. ;)
Na razie nic nie dawałam w funkcji main, bo chyba i tak najpierw musi mi zadziałać kod. Z góry dzięki ;)

Skoro nic nie ma w main to nic nie zadziała.

Chyba, że chodzi Ci o to, że kod się nie kompiluje, a nie że się nie uruchamia.

3
  1. W definicji klasy używasz int zamiast double do określenia Re i Im.
  2. W definicji operatorów pomijasz w ogóle typ, masz jedynie modyfikator const
  3. Nie mieszaj pojęć: liczba zespolona składa się z części rzeczywistej i urojonej a nie zespolonej.
  4. setRzeczywista powinna zwracac void
2

Moje uwagi do pytania o code review:

  1. Kod nie kompiluje się;
  2. Brak testów (nie wymagam pisania testów jednostkowych, ale pusty main?);

Moje uwagi do kodu:

  1. Często zapominasz o typach zmiennych i funkcji;
  2. Nazewnictwo: Konstruktor przyjmuje argumenty a & b. Czy liczby zespolone mają pola a & b?
  3. Funkcja modul:
  • Jak już chcesz przekazywać liczbę zespoloną do tej funkcji, to lepiej przez referencję, ale chyba lepiej jak wykorzystać pola klasy do obliczenia modułu liczby zespolonej?
  • Jakiego typu jest mod?
  • Od kiedy do C++ wprowadzono operator ^ potęgowania?
  1. Pola klasy inicjalizuj za pomocą listy inicjalizacyjnych w konstruktorach:
Complex::Complex(double re, double im) : re_(re), im_(im)
{
}
  1. Operatory arytmetyczne, to dramat, błędnie przekazywane argumenty, brak typów. Operator może wyglądać np. tak:
friend Complex std::operator+(const Complex& c1, const complex& c2)
{
  return Complex(c1.re_ + c2.re_, c1.im_ + c2.im_);
}
6. Przydałby się jeszcze konstruktor jednoargumentowy;
1
  1. operator+ jak i operator- powinieneś zaimplementować jako nie-member funkcje, coby supportować pełną symetrie.
  2. setRzeczywista(int a); a gdzie return type?
0
pingwindyktator napisał(a):
  1. operator+ jak i operator- powinieneś zaimplementować jako nie-member funkcje, coby supportować pełną symetrie.
  2. setRzeczywista(int a); a gdzie return type?

W jaki sposób mam zaimplementować operatory jako nie-member funkcje?

0

Przecież kolega @Sparrow-hawk dobrze Ci napisał. Proponuję zastosować się do uwag wyżej, a potem pytać dalej.

0

Bo w takich deklaracjach operatorów nie może być więcej niż 1 argument... zaczynam ogarniać :)

0

A mogę zapisać je jako funkcje zaprzyjaźnione?

1

Możesz zaimplementować to jako member funkcje, wtedy jednym z argumentów jest this, drugim jest ten przyjmowany jako argument funkcji. Ale to nie jest dobry pomysł i nie chcesz tak robić.
Możesz zrobić z tych operatorów funkcje zaprzyjaźnione, ale znowu - to nie jest dobry pomysł. Polegaj na publicznym interfejsie, nie na szczegółach implementacji. Oszczędzi Ci to refactoringu w przyszłości.

0

Program już się uruchamia, tylko dla każdej liczy moduł wynosi 0. Gdzie jest błąd?

#include <iostream>
#include<cmath>

using namespace std;

class Zespolona{
    double rzeczywista;
    double urojona;
    double mod = sqrt(rzeczywista*rzeczywista+urojona*urojona);
public:
    Zespolona();
    Zespolona(int a, int b);
    ~Zespolona();
    double getRzeczywista();
    void setRzeczywista(int a);
    double getUrojona();
    void setUrojona(int b);
    double getModul();
    friend Zespolona operator+(const Zespolona & zespolona1, const Zespolona & zespolona2);
    friend Zespolona operator-(const Zespolona & zespolona1, const Zespolona & zespolona2);
};
Zespolona::Zespolona(){
    rzeczywista = 1;
    urojona = 1;
}
Zespolona::Zespolona(int a, int b){
    rzeczywista = a;
    urojona = b;
}
Zespolona::~Zespolona(){
    cout<<"Koniec"<<endl;
}
double Zespolona::getRzeczywista(){
    return rzeczywista;
}
double Zespolona::getUrojona(){
    return urojona;
}
void Zespolona::setRzeczywista(int a){
    rzeczywista = a;
}
void Zespolona::setUrojona(int b){
    urojona = b;
}
double Zespolona::getModul(){
    return mod;
}

Zespolona operator+(const Zespolona & zespolona1, const Zespolona & zespolona2)
{
    Zespolona zesp1;
    zesp1.rzeczywista = zespolona1.rzeczywista+zespolona2.rzeczywista;
    zesp1.urojona = zespolona1.urojona + zespolona2.urojona;
    return zesp1;
}
Zespolona operator-(const Zespolona & zespolona1, const Zespolona & zespolona2)
{
    Zespolona zesp1;
    zesp1.rzeczywista = zespolona1.rzeczywista-zespolona2.rzeczywista;
    zesp1.urojona = zespolona1.urojona - zespolona2.urojona;
    return zesp1;
}

int main()
{
    Zespolona z1(15, 9);
    Zespolona z2(6, 8);
    Zespolona z3, z4;
    z3 = z1+z2;
    cout<<z3.getRzeczywista()<<"+"<<z3.getUrojona()<<"i"<<endl;
    z4 = z1-z2;
    cout<<z4.getRzeczywista()<<"+"<<z4.getUrojona()<<"i"<<endl;
    cout<<"Modul liczby z2: "<<z2.getModul()<<endl;
    return 0;
}
1

double mod = sqrt(rzeczywista*rzeczywista+urojona*urojona); inicjalizujesz mod w złym miejscu. Powinieneś robić to wtedy, kiedy znasz już liczby rzeczywista i urojona, tj:

  • Zespolona::Zespolona()
  • Zespolona::Zespolona(int a, int b)
  • Zespolona::setRzeczywista(int a)
  • Zespolona::setUrojona(int b)

W operatorach + i -- bardzo nieładnie to robisz: próbujesz ręcznie ustawić pola rzeczywista i urojona, nie ustawiasz jednak wartości mod. Zastosuj się do mojej uwagi o prywatnych funkcjach a rozwiązesz ten problem.

0

Czyli lepiej po prostu nie używać funkcji zaprzyjaźnionych? I mam jeszcze pytanie, tylko się nie denerwuj... Jak nie masz ochoty, to nie odpowiadaj :D
Czyli mam zastosować prywatne funkcje, czyli np. ```
double Zespolona::Suma()


Ogólnie to mój kod już działa i dzięki wielkie za pomoc, ale chcę wiedzieć na przyszłość co robić lepiej i  jak to na prawdę powinno wyglądać. Zdaję sobie sprawę, że mój kod jest dosyć chaotyczny
2

Nie powinieneś używać friend funkcji - tylko tyle.
Takie operatory powinny wyglądać tak:

Zespolona operator+(const Zespolona & zespolona1, const Zespolona & zespolona2)
{
    Zespolona zesp1(zespolona1.rzeczywista+zespolona2.rzeczywista, zespolona1.urojona + zespolona2.urojona);
    return zesp1;
}

Fajnie, że działa, ale w kodzie jest jeszcze co najmniej kilka błędów - zarówno "błędów w sztuce" jak i zwykłych bugów.

1 użytkowników online, w tym zalogowanych: 0, gości: 1