68

There is a major bug/issue, and also stringing together three very different recommendations here. Bug/issue in the Game-Of-Life you are supposed to scan the entire board, and only then apply the changes. You are applying changes part-way through the process (as you check each cell, you change its state). So, if you change the cell in one location, when ...


55

Two major suggestions: First, let's try speeding everything up by taking advantage of the nature of Conway's Game of Life and the way C++ handles pointers. When you evaluate Conway's Game of Life, it's a two part process. First you copy all of the entries into a new array, then you do your computation referring to that new array, and update the original ...


22

One major thing: do not pass C-arrays to functions as they will decay to a pointer. In C++, you should just use storage containers (from the standard library or Boost) instead of C-arrays, which are more proper for passing storage data to functions (they will not decay to a pointer). You may consider a 2D std::vector of bools: std::vector<std::vector&...


22

const int gridsize = 75; //Making this a global constant to avoid array issues. It's good convention to use all capitals for global constants, for example: const int GRIDSIZE = 75; And now that it looks like a global constant, you can drop the the comment as it became pointless. Instead of: if(grid[a][b] == true){ you can write simply: if (grid[a][b]) ...


20

If you're going to do it in C++, then you might as well use a class. For example, if grid were a data member of a class, and the various functions were methods of the same class, then you wouldn't need to pass the grid as a parameter to each of the various functions methods. a and b aren't bad variable names, but i and j would be more conventional names to ...


18

You're right, there's an alternate way to do this, but, first, some Java standards: 1-liners should have braces. This is a code-style that is common to many langauges because it is more maintainable, and leads to fewer future bugs. Lines like: if (x < 0 || y < 0 || x > getWidth() - 1 || y > getHeight() -1) return; should be: if (x < 0 || y ...


17

First of all, this should be obvious, but when you have performance issues in code that involves a tight inner loop, you want to simplify that loop as much as you can. If you can save one cycle in the loop by spending a dozen or a hundred cycles somewhere else, do it, because every cycle in the loop gets multiplied by 5000². Also, what you really want ...


15

This is excellent code for a Python novice. To address your questions: Rather than x and y, try naming your variables row and col. Then it wouldn't feel unnatural to write self.state.board[row][col]. In my opinion, the .neighbour() function would be better in the State class, since you're counting neighbours of a cell within that state. Your code is easy ...


13

A few suggestions: Each cell in the Game of Life has exactly two states: dead or alive. Unless you plan to implement -1 as undead, I'd suggest you use booleans instead of integers to indicate the state. The int i[] is a bit odd; i is normally used for indices, not for whole rows. In countSurrounding you are using Exceptions to count the surrounding living ...


13

I'll just be cheeky and post a slightly modified version of my SO answer here. So first things first, you want to get rid of the loops. They are slow to execute. The first loop: for x in range(rows): for y in range(cols): if Z[x][y] == 1: if (N[x][y] < 2) or (N[x][y] > 3): Z[x][y] = 0 ...


13

First of all, if the purpose of the getTile method is checking if a position is alive or not, call it isAlive. It makes the code a lot easier to understand that way. It will help to encapsulate the possible directions in an enum: enum Direction { NORTHWEST(-1, -1), NORTH(0, -1), NORTHEAST(1, -1), EAST(1, 0), SOUTHEAST(1, 1), SOUTH(0,...


12

So there's a lot to go though in here. I think that as a beginner project you can be proud of your code... there are a number of things I would change, but, on the whole, it is systematic, logical, and well-formatted. In my opinion this is far more preferable than 'clever' code that is hard to read and understand... So, choosing some things that you should ...


12

Your code is pretty good, and there's not much I'd change. However I would change it in the following ways: It's standard to use UPPER_SNAKE_CASE for Enum values. It's preferred to use typing.Sequence over typing.List to annotate arguments. When you have to comment what a variable is, it indicates you've chosen a bad name. Instead use rows over m. Either ...


11

One rules of thumb to write idiomatic C++ is: Use references where you can and pointers where you have to. It's especially true for argument passing to a variable. In your case, you have the choice, so use references, for all the reasons cited by Ilya and also just because you won't have any * to put everywhere in your function. Your code might be ...


11

You defined depth as 3, but, you're only using 2 strings (one for the alive-or-dead, and the second for the neighbours-count). Instead of storing cell-state as an array of strings, define a class (or perhaps a struct): enum State { Alive, Dead } class CellState { internal State State { get; set; } internal int Neighbours { get; set; } ...


11

Cell The first thing I would do is actually not switch to a bool grid but actually make a class that represents a single cell. I'm going to call it Cell because I'm not that creative. The cell would handle the old and new flags to start with but it would also let you easily add logic for age (some GOL implementations color the cells based on their age). ...


10

Some quick comments: Syntax: coord[inate], not "cord". PascalCase is for public properties/fields, local variables should use camelCase: int xCoord = 9001; I spot a bug //Create copy of cells since all changes must be done simultaneously CellCollection copy = cells; I do not think this does what you think it does. Cache calculations Don't redo this 3 ...


10

You have an interesting implementation. I like your approach into making it "boundless" (that it could expand as-needed). I also like how you use "streams" to compute the values lazily. And some other minor gems. There are of course some things that you could consider in order to make the design better. 1. Separation of Concerns The most important thing I ...


9

I see several issues with the initialization, aside from the obvious gigantic literal array. Ambiguous element IDs: You concatenate x.toString() + y.toString() for the element IDs. When you refer to ID 121, is that (12, 1) or (1, 21)? Namespace pollution (global variables): alive, counter, xcor, ycor, and idAssign do not need to be in the global scope. ...


9

Any idea's on how I can improve on it? The biggest improvement I can think of would be to avoid storing and processing the entire grid. Although there are exceptions, generally Conway's game of life patterns have a lot of empty space. At present your program is spending a lot of time and memory on this empty space. I would recommend just storing the live ...


9

Naming Manifest constants typically use _ for space in Java. So NANOSPERSECOND for example should be NANOS_PER_SECOND. Also I believe that the name MainLoop is quite misleading as a name because the class isn't actually your main loop but rather a timing helper. Avoid unnecessary conversions You don't need the double precision here: public final static ...


9

This is very important: Your algorithm is wrong. Do not ask for help with style or efficiency or anything else like that until you get the algorithm right! Hint: you should not be updating the grid at the same time as you are reading from it. Get the code compiling, then get it right, and then get it elegant and then get it fast. Always that order. So ...


8

Second one is better. Do not use pointers if it is possible to avoid them. For example, in the beggining of the function update(Board *board) you have to check that board!=0 (to be sure, that your program will not crash). But in the function update(Board & board) you are sure, that board exists. So second function is better.


8

Critter Your Point class is, as @RoToRa pointed out, underused. This constructor: Point location; public Critter(int x, int y){ location = new Point(x,y); } Could be simplified to this: Point _location; public Critter(Point location){ _location = location; } Also in this snippet: /** * This method computes the new value of location(which can ...


8

You always need to know where you want to go. Actions shouldn't be split up to much. If you are going to set different functions for different actions (which is something good), they should to be "stand alone". Also you need to "Recycle" what you already have. With something like this Private Const XLength As Long = 100 Private Const YLength As Long = 100 ...


8

Flawed Logic Dirk Reichel makes a good point that the logic for DetermineNextTickState isn't expressed correctly and should be If numNeighbours = 2 Or numNeighbours = 3. Additionally, this: If x <> y Then If currentvalue = 1 Then numNeighbours = numNeighbours + 1 Else isAlive = (...


8

Performance StdDraw.show(int) If you take a look at the documentation for StdDraw.show(int), you'll notice the following: It also speeds up drawing a huge number of shapes (call {@code show(0)} to defer drawing on screen, draw the shapes, and call {@code show(0)} to display them all on screen at once). With that in mind, let's take a look at your drawing ...


8

Turn your warnings on! I would always compile with -Wall -Wextra -pedantic -pedantic-errors :) Let's go through them: Game.cpp:153:37: warning: missing field 'a' initializer [-Wmissing-field-initializers] SDL_Color textColor = { 0, 0, 0 }; ^ There is an alpha component in SDL_Color, which you forgot. To avoid this, ...


7

To reduce memory usage and increase performance I recommend you consider a sparse matrix approach. Instead of having an entire array of mostly empty cells, you can just keep a list of cells that are live. As a cell changes state, you update the display for that cell and the list for the surrounding cells. Edit: I will describe it in more detail as I have ...


7

To address all your questions in one go, you could consider a very simple implementation using only a couple of functions and the set data structure: def neighbors(cell): x, y = cell yield x - 1, y - 1 yield x , y - 1 yield x + 1, y - 1 yield x - 1, y yield x + 1, y yield x - 1, y + 1 yield x , y + 1 yield x + 1, y +...


Only top voted, non community-wiki answers of a minimum length are eligible