random walk problem, correct format

[HomesickAlien]

Hi all,

If it's OK, I post this again (with indentation now). My question was if anyone has suggestions for improving the code or correcting some big mistakes. If it's not OK to post this twice, just ignore it. Also the graphics output is still horrible with this stupid dislin package. Also thanks for ne555 for pointing out one former mistake in the code.

//Random Walk Simulator, scatter plot or bar graph
#include<iostream>
#include "dislin.h"
#include<stdlib.h>
#include<math.h>
#include<time.h>
using namespace std;

const int matrixSize = 600;

int main ()
{
  float histogram[matrixSize] = {0}; //set histogram values to zero
  float gridPositions[matrixSize];  // number of grid points
  int numberOfRealizations = 10000; //number of Monte-Carlo steps
  int numberOfSteps = matrixSize;   //number of steps per run
  int offset = matrixSize / 2;      //starting point
  srand(time(NULL)); //initialize random number generator

    for (int loop; loop < matrixSize; loop++)
    gridPositions[loop] = loop - offset;  //grid points
      for(int outerLoop = 0; outerLoop < numberOfRealizations; outerLoop++)
          {
              double position = offset; //starting point
                  for(int innerLoop = 0; innerLoop < numberOfSteps; innerLoop++)
                     {
                       position = position + 4.0 * (rand()/double(RAND_MAX) - 0.5);
                       histogram[int(position)] += 1;
                     }
          }
    metafl("XWIN");
    int iPoints = 0; // 1: scatter plot, 0 or other value: bar plot

    if (iPoints == 1)
    {
        qplsca(gridPositions, histogram, matrixSize); //plots symbols at data values
    }
  else
  {
      const int binSize = 20;  //number of histogram values in each bin
      const int smallMatrixSize = matrixSize / binSize; //number of bars in plot
      float smallHistogram[smallMatrixSize] = {0};
      cout << smallMatrixSize << endl;

      int binNumber = 0;
      for(int outerLoop = 0; outerLoop < matrixSize - binSize; outerLoop += binSize)
        {
            for(int innerLoop = 0; innerLoop < binSize; innerLoop++)
            smallHistogram[binNumber] = smallHistogram[binNumber] + histogram[binNumber * binSize + innerLoop];
            binNumber++;
        }
      qplbar(smallHistogram, smallMatrixSize); //bar graph of histogram
  }
}

[JLBorges]

#include<stdlib.h>
#include<math.h>
#include<time.h> 

Get rid of these (they may or may not be available). Instead, use C++ headers.

#include<cstdlib>
#include<cmath>
#include<ctime> 

const int matrixSize = 600;

int main ()
{
    float histogram[matrixSize] = {0}; //set histogram values to zero
    // ... 

Consider using std::vector<> instead of an array. Prefer using double as the floating point type (unless there is a specific interoperability/space requirement).

const int matrixSize = 600;

int main ()
{
  float histogram[matrixSize] = {0}; //set histogram values to zero
  float gridPositions[matrixSize];  // number of grid points
  int numberOfRealizations = 10000; //number of Monte-Carlo steps
  int numberOfSteps = matrixSize;   //number of steps per run
  int offset = matrixSize / 2;      //starting point
   ...

Consider writing the program in such a way that parameters can be changed easily, without having to modify code and recompile. For instance, these could be passed as command-line parameters, or picked up from a configuration file or environment variables.


position = position + 4.0 * ... Avoid use of magic numbers in code


int iPoints = 0; // 1: scatter plot, 0 or other value: bar plot

Consider using an enum here. Prefer expressing yourself in working code; use comments only when code is incapable of saying what you want to say.

[HomesickAlien]

Thanks. Seems there is still lots of work to be done here...