4

I have created a class called DNA, having a no argument constructor and two member functions namely initialize() and show(). The problem is when I create an array using new operator and call the initialize function on every object using a for loop, instead of getting different string in the member variable "genes", I am getting the exactly the same set of characters (array) in the genes in every object in the array. Although I seed the srand() function before initialization of the string, there is no effect seen of it.

The code below.

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

string sampleSpace("ABCDEFGHIJKLMNOPQRSTUVWXYZ abcdefghijklmnopqrstuvwxyz");

class DNA {
private:
    int length;
    char *genes;

public:
    DNA() {
        length = 0;
        genes = new char[length];
    }

    void initialize(int len) {
        srand(unsigned(time(NULL)));
        this -> length = len;
        delete genes;
        this -> genes = new char[length];

        for (int i = 0; i < length; i++) {
            *(genes + i) = sampleSpace.at(rand() % sampleSpace.length());
        }
    }

    void show() {
        for (int i = 0; i < length; i++) {
            cout<<*(genes + i);
        }
        cout<<endl;
    }
};

int main() {
    DNA *dna = new DNA[10];
    DNA *temp = dna;
    for (int i = 0; i < 10; i++) {
        (*temp).initialize(10);
        temp++;
    }
    temp = dna;
    for (int i = 0; i < 10; i++) {
        (*temp).show();
        temp++;
    }
    return 0;
}
  • So many leaks... have you heard of `std::vector`? – Guillaume Racicot May 15 '19 at 15:19
  • 1
    you call srand multiple times, you should call it once – Andrew Kashpur May 15 '19 at 15:20
  • Modern C++ programs shouldn't be written this way. There is `std::vector` -- please use that instead of `new[]`. – PaulMcKenzie May 15 '19 at 15:25
  • Possible duplicate of [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – Richard Critten May 15 '19 at 15:25
  • @PaulMcKenzie I could not understand. where should I use std::vector instead of new. apologies for mistunderstanding. – Hardik Sharma May 15 '19 at 15:28
  • 1
    `char *genes;` would be `std::vector genes;` then `genes = new char[length];` would be `genes.resize(length);`. No need for `delete[]`. You could also throw away `length`, since a vector knows its own size by the `genes.size()`. So many things become simpler, and frankly, faster since you wouldn't constantly be calling on the allocator each and every time `initialize` is called. – PaulMcKenzie May 15 '19 at 15:31

2 Answers2

1

You should use the new random API and use a proper random engine:

class DNA {
private:
    int length;
    std::unique_ptr<char[]> genes;

    static std::default_random_engine random;

public:
    DNA() : length{0}, genes{} {}

    void initialize(int len) {
        this-> length = len;
        this-> genes = std::make_unique<char[]>(length);

        std::uniform_int_distribution<std::size_t> distribution{0, sampleSpace.size() - 1};
        for (int i = 0; i < length; i++) {
            genes[i] = sampleSpace.at(distribution(random));
        }
    }

    void show() {
        for (int i = 0; i < length; i++) {
            cout<<genes[i];
        }
        cout<<endl;
    }
};

This will initialize a std::default_random_engine and use a proper number distribution. Also, I changed the code for unique pointer.

Here's a live example.

Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141
  • To be honest, I have never heard of unique pointer before. The code that you used, I literally couldn't understand. Is there any reference where I could learn more about std::default_random_engine and std::uniform_int_distribution – Hardik Sharma May 15 '19 at 15:33
  • @HardikSharma yes, on [cppreference](https://en.cppreference.com/w/cpp/numeric/random) – Guillaume Racicot May 15 '19 at 15:35
  • @HardikSharma I don't think it's a good idea. You can join the cpplang slack lf you wish to communicate directly. Also don't hesitate to ask other questions on stack overflow. I usually tend to hang around these places. – Guillaume Racicot May 15 '19 at 15:41
  • when I ran the program , it threw following errors, C:\Users\Admin\Desktop\programs\c++\genetic algo to evolve a sentence\main.cpp|15|error: 'unique_ptr' in namespace 'std' does not name a template type and a numerous more that were no brainer. this was the first one. can you specify what's going on my compiler. – Hardik Sharma May 15 '19 at 15:43
  • In this case wouldn't a vector make the job easier than the unique_ptr? This seems like exactly the kind of task it exists for. – Wutz May 15 '19 at 15:43
  • 1
    @Wutz yes I know, but I didn't want to change the code too much. – Guillaume Racicot May 15 '19 at 15:44
  • @HardikSharma comments are not meant for error messages. – Guillaume Racicot May 15 '19 at 15:45
  • @HardikSharma I updated the answer with corrected code and live example. – Guillaume Racicot May 15 '19 at 15:51
  • @GuillaumeRacicot The program works just fine for generating the random strings. However I get the same set of the string array every time I run the program. Can you help me out ? – Hardik Sharma May 16 '19 at 08:51
  • @GuillaumeRacicot also when I read the reference of random header file that you provided, I could understand much of the things. There was a class (I suppose) namely std::default_random_engine that needs a std::random_device object to initialize its obect, the problem is when I ran this code in my program certain errors occured , one of them was like `class std::random_device has no member named 'generate' ;` – Hardik Sharma May 16 '19 at 08:59
1

To piggyback on the answer given, here is the equivalent answer, but using std::vector and std::generate:

#include <iostream>
#include <algorithm>
#include <ctime>
#include <string>
#include <vector>
#include <random>

std::string sampleSpace("ABCDEFGHIJKLMNOPQRSTUVWXYZ abcdefghijklmnopqrstuvwxyz");

class DNA 
{
    private:
        std::vector<char> genes;

    public:
        void initialize(int len) 
        {
            static std::default_random_engine random;
            genes.resize(len);
            std::uniform_int_distribution<size_t> distribution{0, sampleSpace.length()-1};
            sampleSpace.at(distribution(random));
            std::generate(genes.begin(), genes.end(), [&] () 
                          { return sampleSpace.at(distribution(random)); });
        }

        void show() 
        {
           for (auto& v : genes)
                std::cout << v;
            std::cout << "\n";
        }
};

int main() 
{
    DNA dna[10];
    for (int i = 0; i < 10; i++) 
        dna[i].initialize(10);
    for (int i = 0; i < 10; i++) 
        dna[i].show();
}

Live Example

Note that length is also no longer needed.

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • Thanks for the program. It worked fine in generating the random string vectors on top of sampleSpace and made a lot of things easier. However there still a problem : the same set of string vector is generated every time I run the program. I'd be glad I you could help me generating the random set of string vector on every other execution. – Hardik Sharma May 16 '19 at 09:15