Wator planet program

1. The first thing that I see when I look at your code is that it is UGLY. I appreciate it doesn't affect how well the code works, but it does affect how easy it is to read and if it is hard to read then you are more likely to miss important issues. When you come back to this code in three months it will be like you are new to it. So my first recommendation is adopt a common style for the language you are using and make sure you stick to it. Look at other people's C# code on here, yours looks more like Java than C#.

2. You have defined neighbourIndex at the top and assigned an initial value to it a few lines later. It is a good idea to always assign a value to a variable when you declare it. To be honest this is less important in C# that C++, but you can still get in nasty situations when you assume an object has been assigned a value and it hasn't.

3. You have used a nice, descriptive name like neighbourIndex and then gone and used i and j. If I have read the code correctly these are the x and y co-ordinates of a cell. So why not use x and y? Also since you have already used the Point class would it not be a good idea to use the Point class to store i and j in?

4. In C#, as far as I understand, there is no difference in the speed of execution of x++ and ++x, but in other languages with other compilers this may not be the case (C++ for instance) so I would recommend that if you are doing x++; you get into the habit of doing ++x; because this can sometime be faster for non simple data types. HOWEVER in this case it would be better to do neighbors[neighborIndex++] = new Point(i, j);

5. I agree with user673679 that there could be a significant amount of optimisation here, you are doing something very similar 4 times, that should scream out to you to be optimised.

And finally a positive, your code is littered with comments and that is brilliant, keep it up. If you decide to make any changes then remember to check that your comments still make sense. Also finish the job off with a nice descriptive comment at the top of the functions to explain what the function does, what the parameters are, what ranges they take and what the function returns. If you really want to go all out, describe the expected pass and fail conditions so that it becomes easy to write tests for the function.

Hope that helps

It's hard to know exactly what would help without a seeing more code, but here are some things to consider:

If neighbors is a pre-allocated array, do you need new Point(i,j) or can you simply do neighbors.X = i; neighbors.Y = j? You might consider using a standard collection type instead.

It seems like you're using different object types for animals. It would be much simpler to use an enum: enum SquareType { Empty, Fish, Shark }; and then you could compare values instead of using IsInstanceOfType. This would also eliminate the special-case need to check if grid cells or type were null.

There's also quite a lot of duplicated functionality there. All of your direction checks could be merged into a function that took an xOffset and yOffset parameters. Then you could do something like:

int neighborIndex = 0; neighborIndex += CheckNeighbor(-1, 0, type, position); // left neighborIndex += CheckNeighbor(1, 0, type, position); // right neighborIndex += CheckNeighbor(-1, 0, type, position); // up neighborIndex += CheckNeighbor(1, 0, type, position); // down