100ричная система счисления и наличие ляпов в программе

Вот уже некоторое время занимаюсь решением задач на классы и на этот раз, решая задачу про числа в 100ричной системе счисления, данная задача полностью решена, все тесты проходят.

Но было сказано, что в программе есть некоторые "ляпы". Нкоторые найдены, исправлены. Но есть еще. Хотя программа работает, причин для утечки памяти,ну или другой проблемы, я не вижу. Оператор присвоения, вроде бы, записан верно, в перегрузке операторов в качестве аргумента записываю не объект, а его адрес, т.о копии объекта в итоге не создается, утечки нет, консруктор копий не нужен, его не записываю. Деструктор записан.

Какие тут могут быть ляпы не понимаю. Вроде бы нормально изучил классы за всё время. Если Вы знаете, давайте обсудим.

#include <iostream>
#include <cmath>
using namespace std;


class Sto{

    int *data;
    int size, sign;
    public:
    
    Sto(int asize)
    { 
        size=asize; 
        data = new int[size];
        sign=1;
    }
    
    ~Sto()
    {
        delete[] data;
        size = sign = 0;
    }
        
    Sto& operator=(const Sto& ob)
        {
            
            if (this == &ob)
            return *this;
            
            if (data) delete[] data;
            
            size=ob.size;
            sign=ob.sign;
            
            data=new int[size]; 
            for(int i=0;i<size;i++)
            data[i]=ob.data[i];
    
            return *this;
        }
        
    friend Sto operator+(const Sto &x, const Sto &y);
    friend Sto operator*(const Sto &x, const int &chislo);
            
    void scan()
    {   
        int x_p=0;
        cout<<"Vvedite "<<size<<" cifri\n";

        for(int i=0; i<size; i++)
            cin>>*(data+i);

        while(data[x_p]==0) x_p++; 

        sign = (data[x_p] > 0) - (data[x_p] < 0);
        data[x_p]=abs(data[x_p]);
    }
        
    void print(){
        int x_p=0;
        while(data[x_p]==0) x_p++; 

        cout<<sign*data[x_p]<<" ";
        for(int i=x_p+1; i<size; i++) 
            cout<<data[i]<<" ";
    }
};

Sto operator+(const Sto &x, const Sto &y)
{
    Sto z(max(x.size,y.size)+1);
    int x_i=x.size-1, y_i=y.size-1, z_i=z.size-1, tmp1=0, tmp2=0, major=1, x_p=0, y_p=0;
    
    while(x.data[x_p]==0) x_p++; 
    while(y.data[y_p]==0) y_p++; 
    
    if(x.sign!=y.sign && x.size - x_p <= y.size - y_p)
    {
        if(x.size - x_p < y.size - y_p) major = -1;
        else
        {
            for(int i=x_p, j=y_p; i<x.size; i++, j++) 
                if(x.data[i] < y.data[j]) { major = -1; break; }
        }
    }

    if(x.sign == y.sign && x.sign==-1 || x.sign==-1 && major==1 || y.sign==-1 && major==-1) z.sign=-1;

    while(z_i>=0)
    {
        tmp1 = 0; z.data[z_i] =0;
        
        if(x_i>=x_p) z.data[z_i] += major * x.data[x_i];
        if(y_i>=y_p) z.data[z_i] += major * (x.sign * y.sign) * y.data[y_i];
    
        if(x.sign==y.sign)
        {
            if(y_i>=y_p-1 && y_i<y.size-1) tmp1 += y.data[y_i+1];
            if(x_i>=x_p-1 && x_i<x.size-1) tmp1 += x.data[x_i+1];
            z.data[z_i]+= tmp1/100;
    
            if(z_i==1 && z.data[1]>=100) 
                z.data[0] = z.data[1]/100;
            z.data[z_i] %=100;          
        }
        else
        {
            z.data[z_i]-=tmp2;  
            if(z.data[z_i] < 0) {z.data[z_i]+=100; tmp2 = 1;} else tmp2=0;
        }
    
        x_i--; y_i--; z_i--;
    }   
    
return z;
}

Sto operator*(const Sto &x, const int &chislo)
{

    Sto z(x.size + 1 + log(abs(chislo))/log(100) + 1);
    int x_i=x.size-1, z_i=z.size-1, temp=0, x_p=0;
    
    z.sign=1; if(x.sign * chislo < 0) z.sign=-1;
    
    while(x.data[x_p]==0) x_p++; 
    
    while(z_i>=0)
    {
        z.data[z_i] = temp;
        if(x_i>=x_p) z.data[z_i] += x.data[x_i]*abs(chislo);
    
        temp = z.data[z_i]/100;
        z.data[z_i] = z.data[z_i]%100;
        
        x_i--; z_i--;
    }
    
return z;
}

int main()
{
    int size_x, size_y, number;
    
    cout<<"Vvedite razmer 1-go massiva"<<endl;
    cin>>size_x;
    Sto ob1(size_x);
    ob1.scan();
    
    cout<<"\nVvedite razmer 2-go massiva"<<endl;
    cin>>size_y;
    Sto ob2(size_y);
    ob2.scan();
    
    Sto ob3(max(size_x,size_y)+1);
    ob3 = ob1 + ob2;

    cout<<"\nVvedite konstantu"<<endl;
    cin>>number;

    Sto ob4(size_x + 1 + log(abs(number))/log(100)+1);
    ob4 = ob1 * number;
    
    cout<<"\nSlojeniye dvux storichnix chisel: "; ob3.print();
    cout<<"\nUmnojeniye ob1 na konstantu: "; ob4.print();
    
return 0;
}

Ответы (1 шт):

Автор решения: gbg

Я вижу семь проблем, как минимум:

Первая - вы используете сырой указатель вместо std::unique_ptr. Использовение умного указателя автоматически избавляет вас от необходимости писать деструктор. Кроме того, оно блокирует автоматически созданный конструктор копирования.

Вторая - ваш деструктор делает бесполезную работу. Присваивания sign = size = 0 не несут смысла, так как следующим шагом эти значения будут уничтожены при освобождении памяти после деструктора.

Третья - ваш деструктор не объявлен виртуальным. Если кто-то унаследуется от вашего класса, цепочка деструкторов будет работать неверно. Следует или объявить деструктор виртуальным, или дописать final в декларацию класса:

class Sto final
{

Если оставить код как есть - это приведет к проблемам в дальшейшем.

Четвертая - для размеров в C++ имеется size_t, следует использовать его, а не int.

Пятая - от написания коструктора копирования можно избавиться, если хранить число в std::vector. Это другой способ решить проблему номер 1, с сырым указателем.

Шестая - префиксные икременты и декременты всегда более предпочтительны, чем постфиксные, лучше использовать их (++i лучше i++)

Седьмая - инициализацию полей следует делать не в теле конструктора, а в списке инициализации полей:

Sto::Sto(const size_t size)
    :data(new int[size])
    ,size(size)
    ,sign(1)

Знак (sign) следует инициализировать даже раньше - в объявлении класса. Чем раньше значение инициализировано, тем лучше.

Дополнительно, можно отметить ряд проблем с форматированием кода. Как правило, форматирование кода является довольно субъективной штукой и регламентируется гайдом по код-стайлу компании. Однако, некоторые примененные вами приемы считаются нежелательным:

int x_i=x.size-1, y_i=y.size-1, z_i=z.size-1, tmp1=0, tmp2=0, major=1, x_p=0, y_p=0;

В данном случае, мы имеем сразу две проблемы:

Первое. Довольно длинная строка, которую на некоторых мониторах придется скроллить, чтобы просмотреть.

Второе. Несколько объявлений на одной строке. Большинство гайдов по стилю регламентируют принцип "одна строка - одно объявление". Это позволяет более явно выделить появление новых сущностей, оставляет место для комментариев и защищает от интересных ошибок вида:

int a, b, c = 4, 5, 1; //если вы думаете 
                       //что в итоге будет 
                       //a = 4, b = 5, c = 1, 
                       //то нет.
→ Ссылка