Program not working for some reason, could someone pls help me fix it

Question

Could someone please help me fix my program and explain why it s not working?

It's supposed to generate n points with 2 coordinates, which are both random numbers. The values themselves are random but have to scale the interval from 0 to some chosen value k. All the points have to be apart from each other by some radius which is taken to be 1.

For some reason my program doesn't even start. When I run it, Windows just says that the program is not responding and is trying to diagnose the problem.

Please simplify your explanation as much as possible since I'm a complete beginner and probably won't understand otherwise. Thanks a bunch in advance.

#include <iostream>
#include <vector> 
#include <cstdlib>
#include <cmath>
#include <fstream>

using namespace std;

int main()
{  
   int n=5;
   int k=100;
   vector<vector<double>> a(n, vector<double> (2));
   srand(132);
   //a[0][1]=k*((float(rand()))/RAND_MAX);
   //a[0][0]=k*((float(rand()))/RAND_MAX);
   for(int i=0; i<n;){  
      a[i][0]=k*((float(rand()))/RAND_MAX);
      a[i][1]=k*((float(rand()))/RAND_MAX);
      for (int j=0; j<n; j+=1){
         if (sqrt(pow((a[i][1]-a[j][1]),2)+pow((a[i][0]-a[j][0]),2))<=1){        
            i=i;
            break;}
         else if(j==n-1){
            cout << a[i][0] << " " << a[i][1] << endl;
            i+=1;}
   }}
   return 0;
}

Answer 1

Your code lacks structure. That's why it is hard to understand, as you now learned even for you.

I think a good start would be to write a class for point and two functions, one for random points and for point distance then all, especially the double loops, will become much easier to read and debug.

Look at this:

#include <iostream>
#include <vector>
#include <cmath>

using namespace std;

struct Point
{
    Point() = default;
    float x;
    float y;
};

float scaled_random(int k)
{
    return k*((float(rand()))/RAND_MAX);
}

float distance(const Point& a, const Point& b)
{
    return sqrt(pow(a.y-b.y,2)+pow(a.x-b.x,2));
}

int main()
{
    int n = 5;
    int k = 100;
    vector<Point> a(n);

    srand(132);

    for (int i=0; i<n; ) {
        a[i].x = scaled_random(k);
        a[i].y = scaled_random(k);
        for (int j=0; j<n; j+=1) {
            if (distance(a[i], a[j]) <= 1) {
                i = i;
                break;
            } else if (j == n-1) {
                cout << a[i].x << " " << a[i].y << endl;
                i += 1;
            }
        }
    }
    return 0;
}

The issue is still the same, but it has now more structure, better formatting and superfluous includes removed.

Maybe you can see the problem yourself much better this way.

Answer 2

The first time through your code i and j will both be zero, this means a[i][1] - a[j][1] and a[i][0] - a[j][0] are zero, this resets i to 0, breaks the loop and starts again resulting in an infinite loop.

Checking i != j fixes the problem:

if (i != j && sqrt(pow((a[i][1] - a[j][1]), 2) + pow((a[i][0] - a[j][0]), 2)) <= 1) {

Your code might be better structured as:

#include <iostream>
#include <vector> 
#include <cstdlib>
#include <cmath>
#include <algorithm>

int main()
{
    int n = 5;
    int k = 100;
    std::vector<std::vector<double>> a(n, std::vector<double>(2));
    srand(132);
    for (int i = 0; i < n; i++) {
        auto end = a.begin() + i;
        do
        {
            a[i][0] = k * ((float(rand())) / RAND_MAX);
            a[i][1] = k * ((float(rand())) / RAND_MAX);
        }
        while (end != std::find_if(a.begin(), end, [&](const std::vector<double>& element) 
            { 
                return sqrt(pow((a[i][1] - element[1]), 2) + pow((a[i][0] - element[0]), 2)) <= 1;
            }));
        std::cout << a[i][0] << " " << a[i][1] << "\n";
    }

    return 0;
}

Using this code only the values before i are checked each time rather than all of the values.

rand should be avoided in modern c++, see Why is the use of rand() considered bad?

As the elements of your vector always have 2 elements it'd be better to use std::pair or std::array.

pow may be quite an inefficient way to square two numbers. The sqrt could be avoided by squaring your distance instead.

Using the above points your code could become:

#include <iostream>
#include <vector> 
#include <cstdlib>
#include <cmath>
#include <algorithm>
#include <array>
#include <random>

using point = std::array<double, 2>;

double distanceSquared(const point& a, const point& b)
{
    auto d0 = a[0] - b[0];
    auto d1 = a[1] - b[1];
    return d0 * d0 + d1 * d1;
}

int main()
{
    int n = 5;
    int k = 100;
    std::vector<point> a(n);
    std::random_device rd;
    std::mt19937_64 engine(rd());
    std::uniform_real_distribution<double> dist(0, k);
    for (int i = 0; i < n; i++) {
        auto end = a.begin() + i;
        do
        {
            a[i][0] = dist(engine);
            a[i][1] = dist(engine);
        }
        while (end != std::find_if(a.begin(), end, [&](const point& element)
            { 
                return distanceSquared(a[i], element) <= 1;
            }));
        std::cout << a[i][0] << " " << a[i][1] << "\n";
    }

    return 0;
}