Optymalizacja kodu do programu "Generator liczb losowych z zapisem do pliku z wykorzystaniem parametrów"

Siemanko. Mam pytanie. Czy poniższy skrypt można bardziej zoptymalizować, bo jak do parametrów przekażesz 2cyfrowe wartości liczbowe program się wysypuje. Parametry się przekazuje kiedy aktywuje się skrypt i nie jestem pewny czy prawidłowo przeprowadziłem walidacje parametrów. Potrzebuje szybkiej odpowiedzi.

main.cpp

#include <iostream>
#include <windows.h>

#include "assets/Args.cpp"
#include "assets/generateData.cpp"
#include "assets/saveData.cpp"

#define MAX_ARGUMENTS 2

using namespace std;

int main(int argc, char **argv){
    system("cls");
    
    try{
        Args <int, MAX_ARGUMENTS> arguments(argc, argv);
        int* nums = arguments.get();
        int** data = generateData(nums);

        saveData(data, nums);

        cout << "Done!" << endl;
    }

    catch(const invalid_argument &e){ 
        if(e.what() == "stoi") cout << "Wprowadzono litere zamiast liczby!" << endl;
        else cout << e.what() << endl;
    }

    system("pause");
    return 0;
}

assets/Args.cpp

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

template <typename T = string, int MAX_ARGUMENTS = 1> class Args {
    T * argv = new T();
    int argc;

    T value(string value){
        if(is_same<T, int>::value) return stoi(value);
    }
    
    public:
        Args(int argc, char **argv){
            if((argc - 1) != MAX_ARGUMENTS) throw invalid_argument("Nieprawidlowa ilosc argumentow! Liczba argumentow musi wynosic " + to_string(MAX_ARGUMENTS) + "!"); 

            for(int i = 1; i < argc; i++){
                int value = this->value(argv[i]);

                if(value <= 0) throw invalid_argument("Wartosc nie moze byc mniejsza od zera!");
                this->argv[i - 1] = value;
            }

            this->argc = argc - 1;          
        }

        ~ Args() { delete[] this->argv; }
        T *get() { return this->argv; }
};

assets/generateData.cpp

#include <iostream>
#include <time.h>

int** generateData(int* nums){
    srand(time(NULL));

    int x = nums[0], y = nums[1];
    int** rows = new int* ();

    for(int i = 0; i < x; i++){
        int* cols = new int();

        for(int j = 0; j < y; j++) cols[j] = rand() % 100 + 1;
        rows[i] = cols;
    }

    return rows;
}

assets/saveData.cpp

#include <iostream>
#include <fstream>
#include <string>
using namespace std;

void saveData(int** data, int *nums){
    int x = nums[0], y = nums[1]; fstream f;
    f.open("data.txt", ios::out);

    for(int i = 0; i < x; i++){
        for(int j = 0; j < y; j++)
            f << to_string(data[i][j]) << " ";

        f << "\n";
    }
    
    f.close();
}

bo jak do parametrów przekażesz 2cyfrowe wartości liczbowe

Do jakich parametrów?

[...] program się wysypuje.

To w końcu program trzeba zoptymalizować (aby działał szybciej / używał mniej pamięci), czy naprawić w nim jakiś błąd?

@ArashiYT: W konstruktorze masz zły warunków pętli for. Powinno być <=

Patryk27 napisał(a):

Do jakich parametrów?

Chodzi o argumenty przekazywane z konsoli, czyli np.: main.exe 2 3. Przepraszam, ale często się mylę z tymi pojęciami

To w końcu program trzeba zoptymalizować (aby działał szybciej / używał mniej pamięci), czy naprawić w nim jakiś błąd?

Szczerze to i to. Bo sam program działa. Ale przekazując jej większe wartości to już nie. Tak jakby był błąd, ale o nie wyświetla. Po prostu nie wyświetla się na konsoli tekst Done! ani nie działa instrukcja system("pause");. Zwraca się po prostu czarny ekran. Potem się wyłącza. A obserwując plik, to nic się nie zmieniło. Mam podejrzenie że po prostu błąd jest w assets/generateData.cpp. Tylko nie wiem konkretnie co. Ale przekazując jej np wartości 2 i 3 lub 5 i 5 to działa.

I nie bardzo mogę zrozumieć instrukcji cpp delete. Mógłby mi ktoś szerzej rozjaśnić? Wiem jedynie tyle, że służy to cofania przydział bloku pamięci. I nie jestem pewien czy używając go w destruktorze zrobiłem dobrze. Bo używałem jeszcze go po wykonaniu wszystkich instrukcji w main.cpp i wywalało mi błędy.

int** rows = new int* ();

^ nie powinieneś tutaj w jakiś sposób przekazać jak dużo tych wierszy potrzebujesz? (to samo do kolumn)

Sama klasa Args posiada przynajmniej trzy błędy, gdzie najpoważniejszym jest ten który wskazał @Patryk27. Spodziewasz się, że T będzie tablicą, zwalniasz poprawnie jak tablicę, ale inicjalizujesz miejsce tylko na jeden element. Zrób to wg poniższego pseudokodu

class Args
{
T *val {nullptr};

Args(int argc)
  : val{ new T[argc] }
{
}

~Args()
{
  if(val)
    delete[] val
}
};

Chyba nie zrozumiałeś jak działa new i zarządzanie pamięcią. To jest też źle, z tego samego powodu (i to w dwóch miejscach):

int** generateData(int* nums){
    srand(time(NULL));

    int x = nums[0], y = nums[1];
    int** rows = new int* ();

    for(int i = 0; i < x; i++){
        int* cols = new int();

        for(int j = 0; j < y; j++) cols[j] = rand() % 100 + 1;
        rows[i] = cols;
    }

    return rows;
}

Polecam lekturę: https://dsp.krzaq.cc/post/176/ucze-sie-cxx-kiedy-uzywac-new-i-delete/

Dziękuję. Problem został rozwiązany. Nie jestem dobry w c++, jak widać. Ale dziękuję. Również za to, że dowiedziałem się jakie jeszcze błędy ja popełniłem. Jeszcze raz dziękuję. I życzę miłego dnia. I jeszcze chciałbym zapytać. Czy znacie może dobry kurs do c++? Bo w większości ja sam wszystkiego się uczę. Ale jak widać, mi to nie wychodzi.

Czy znacie może dobry kurs do c++?

Kursów nie znam. Jeśli chodzi o literaturę to był niedawno wątek z wieloma pozycjami do wyboru https://4programmers.net/Forum/C_i_C++/357717-wybor_ksiazki_do_nauki_c