3
\$\begingroup\$

A class Array which contains two data members: a pointer on array of complex numbers and the length of the array. Implement a function for reading the array. The class should have a constructor, destructor and copy constructor. In the main program, enter the array of complex numbers.

main.cpp

#include <iostream>
#include "complex.h"

using namespace std;

class Array{
private:
complex *n;
int length;
public:
Array(void);
Array(int,complex *);
~Array();
Array(const Array &);
void print_array(void);
};

Array::Array(void){
}

Array::Array(int _length, complex *_n){
length=_length;
n=new complex[length];
for(int i=0;i<length;i++){
n[i]=_n[i];
}
}

Array::~Array(){
delete [] n;
n=0;
}

 Array::Array(const Array & _n){
 length=_n.length;
 n=new complex[length];
 for(int i=0;i<length;i++){
 n[i]=_n.n[i];
 }
 }

 void Array::print_array(void){
 for(int i=0;i<length;i++){
 cout<<""<<(n[i]).return_real()<<"+("<<(n[i]).return_imaginary()<<")j"    <<endl;
 }
 }

 int main()
 {
 complex *numbers;
 int length;
 float a,b;
 cout<<"How many complex numbers you will enter? "<<endl;
 cin>>length;
 numbers=new complex[length];
 cout<<"Enter real and imaginary parts of "<<length<<" complex numbers"   <<endl;
 for(int i=0;i<length;i++){
 cin>>a>>b;
 numbers[i]=complex(a,b);
 }
 Array n1(length, numbers);
 n1.print_array();
 Array n2(n1);
 }

complex.cpp

#include "complex.h"

complex::complex(void){
}

complex::complex(float a,float b){
real=a;
imaginary=b;
}

complex::~complex(){
}

float complex::return_real(void){
return real;
}

float complex::return_imaginary(void){
return imaginary;
}

complex.h

#ifndef COMPLEX_H_INCLUDED
#define COMPLEX_H_INCLUDED

class complex{
private:
float real;
float imaginary;
public:
complex(void);
complex(float,float);
~complex();
float return_real(void);
float return_imaginary(void);
};

#endif // COMPLEX_H_INCLUDED

I tested it and it works. I want to know if there is something I can do to improve it.

\$\endgroup\$
1
\$\begingroup\$

The code isn't well indented, but maybe that's only your post on codereview (if it isn't, please consider indenting your code, that helps!)

There's a flaw in your code: you thought about reimplementing Array's copy constructor (which is good), but you didn't think about the copy assignment operator (which is implicitly defined). Currently, this code is legal, and is very likely to cause undefined behaviour (that may or may not "work", you don't know, usually you'll get a segfault when it doesn't):

#include "array.hpp" // let's say you moved the array definition there

void uselessAssignment(const Array& a) {
  Array b;
  b = a; // call to the implicitly defined assignment operator!
  // call to b destructor (you just deleted the memory allocated by a)
}

int main(int, char*[]) {
  complex complexNumbers[42];
  Array a(42, complexNumbers);
  uselessAssignment(a);
  a.printArray();
  return 0;
}

To correct that, either redefine the implicit assignment operator or mark it as delete.

Otherwise that seems like a fine minimal implementation, it could be improved though:
- you forgot a delete in the main, that's not extremely important because the OS will free the memory anyway when the program terminates, but that's not really clean
- i've learnt today that you should avoid prefixing your names with an underscore (stack overflow link)
- in complex.cpp you can use the initializer list to initialize the real and imaginary parts (your code works fine, i just personally find it cleaner):

complex::complex(float a, float b) : 
real(a), 
imaginary(b) {}

- you may want your complex number to use something else than a float to store the real and imaginary parts:

template<class Real = float, class Imaginary = Real>
struct complex {
  //...
  Real real;
  Imaginary imaginary;
};

- you can use std::vector to handle your array rather than allocating/deallocating yourself, it's a bit safer (you wouldn't have the problem with the copy assignment), but i often directly use dynamic allocation myself so i'm not one to talk
- rather than this:

float a, b;
for(int i=0;i<length;i++){
  cin>>a>>b;
  numbers[i]=complex(a,b);
}

you can overload operator>> for complex rather than using the assignment operator:

std::ostream& operator>>(std::ostream& output, complex& number) {
  float a, b;
  output >> a >> b;
  number = complex(a, b);
  return output;
}
// ...
int main(int, char*[]) {
  // ...
  for(int i = 0; i < length ; ++i )
    cin >> numbers[i];
  // ...
}

(you can also declare the operator>> as a friend in complex so that it can directly access its members; when the assignment isn't trivial, it may also slightly improve performance, but not here)
- of course you could define arithmetic operations on complex numbers

...and probably other changes, but that's a rather long list already (and that also depends on what you expect from your classes).

Edit:
Ah something important if you manually handle memory in your class, you may want to define the move constructor and the move assignment operator, that may improve performance in some cases:

Let's say you have some function which doubles each complex number in an array and returns a new array (it doesn't modify the original one), let's say its protoype is Array doubleEachComplex(Array&);
If you use it like this:

Array array;
// ...
array = doubleEachComplex(array);

The compiler will:
- construct array
- do things with array (initialize it for instance)
- call doubleEachComplex
- create a fresh Array object
- call the copy assignment operator of Array (if no move assignment operator is defined)
- delete the fresh Array object (used by the copy)

But the copy assignment operator can't modify the "source" object, so it will usually perform a deep copy. A better solution would be to swap the contents of each array since the second one is a temporary (that's what a move constructor could do).
Actually, in that case an even better solution would have been to modify it in place, but that's not always possible/a good idea.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.