I've made this relatively basic height map generator on my own just to see if I could pull it off because I'm still a newbie when it comes to programming. So my question is: "Is there a fairly obvious way I can optimize it?". I put in effort to make the code as clean as I can write it so any input from a quick review (if you have nothing better to do of course) would be greatly appreciated. Here is the ORIGINAL code:
using System;
using System.Diagnostics;
namespace height_map_generator
{
class Program
{
private static readonly int bufferWidth = 237;
private static readonly int bufferHeight = 90;
private static readonly double nodeProbability = 0.007; //Initial node spawn probability in each point of buffer array. (The higher this number is the more peaks on the map.)
private static readonly double intervalFraction = 0.03; //Number must be between 0 and 1. (The lower this number - the higher the variability and sharpness of terrain.)
private static readonly double maxDistanceFromCentre = Math.Sqrt(Math.Pow(bufferWidth / 2, 2) + Math.Pow(bufferHeight / 2, 2));
private static readonly double distanceInterval = maxDistanceFromCentre * intervalFraction;
private static readonly Random rand = new Random();
private static int[,] mapBuffer = new int[bufferWidth, bufferHeight];
private static int[,] nodePositionValue = new int[bufferWidth, bufferHeight];
private static int[] nodeXposition = new int[bufferWidth * bufferHeight];
private static int[] nodeYposition = new int[bufferHeight * bufferWidth];
private static int nodeCount = 0; //Currently not in use.
static void Main(string[] args)
{
Console.SetBufferSize(bufferWidth, bufferHeight);
Console.ForegroundColor = ConsoleColor.Green;
string elapsedTime = null;
Stopwatch sw = new Stopwatch();
sw.Start();
constructNodes(nodeProbability); // First.
setNodeCoordinates(); // Second.
compareDistancesAndSetEachPoint(); // Third.
sw.Stop();
elapsedTime = sw.Elapsed.ToString();
writeTerrain(); // Final.
Console.SetBufferSize(bufferWidth, bufferHeight + 2);
Console.WriteLine();
Console.Write("Operation completed in: " + elapsedTime);
Console.ReadKey(); //Wait before quitting.
}
private static void constructNodes(double probability)
{
for (int y = 0; y < bufferHeight; y++)
{
for (int x = 0; x < bufferWidth; x++)
{
if (probabilityResult(probability))
{
int nodeAmp = generateNodeAmplitude();
mapBuffer[x, y] = nodeAmp;
nodePositionValue[x, y] = nodeAmp;
}
}
}
}
private static void setNodeCoordinates()
{
int nodes = 0;
for (int y = 0; y < bufferHeight; y++)
{
for (int x = 0; x < bufferWidth; x++)
{
if (nodePositionValue[x, y] != 0)
{
nodeXposition[nodes] = x;
nodeYposition[nodes] = y;
nodes++;
}
}
}
nodeCount = nodes;
}
private static void compareDistancesAndSetEachPoint()
{
for (int y = 0; y < bufferHeight; y++)
{
for (int x = 0; x < bufferWidth; x++)
{
if (spaceAvailable(x, y))
{
int n = 0;
double minimumDistance = 0;
int nodeAmplitude = 0;
foreach (int i in nodeXposition)
{
if (n == 0)
minimumDistance = Math.Sqrt(Math.Pow(x - nodeXposition[n], 2) + Math.Pow(y - nodeYposition[n], 2));
else if (minimumDistance > Math.Sqrt(Math.Pow(x - nodeXposition[n], 2) + Math.Pow(y - nodeYposition[n], 2)))
{
minimumDistance = Math.Sqrt(Math.Pow(x - nodeXposition[n], 2) + Math.Pow(y - nodeYposition[n], 2));
nodeAmplitude = nodePositionValue[nodeXposition[n], nodeYposition[n]];
}
n++;
}
setAmplitude(x, y, minimumDistance, nodeAmplitude);
}
}
}
}
private static void writeTerrain()
{
for (int y = 0; y < bufferHeight; y++)
{
for (int x = 0; x < bufferWidth; x++)
{
Console.SetCursorPosition(x, y);
Console.Write(mapBuffer[x, y]);
}
}
}
/*******************************************************************************/
private static void setAmplitude(int x, int y, double distance, int nodeAmplitude)
{
int k = 1;
while (true)
{
if (distance <= k * distanceInterval)
{
//---------------------------------------COSMETIC 'IF' STATEMENT----------------------------------------- (Should be removed when using the data.)
if (k + nodeAmplitude <= 9)
mapBuffer.SetValue(nodeAmplitude + k, x, y);
else
mapBuffer.SetValue(9, x, y);
//-------------------------------------------------------------------------------------------------------
break;
}
k++;
}
}
private static bool spaceAvailable(int x, int y)
{
if (nodePositionValue[x, y] == 0)
return true;
else
return false;
}
private static bool probabilityResult(double percent)
{
if (percent <= 0 || percent > 1)
throw new ArgumentOutOfRangeException("Probability must be between 0 and 1.");
return rand.NextDouble() <= percent;
}
private static int generateNodeAmplitude()
{
int num = rand.Next(1, 4);
return num;
}
}
}
5 Answers 5
A couple quick things:
If you are using while loops for iterations, just use for loops.
//I have to look in three places to understand how the loop works
int x = 0;
while(x < 10){x++;}
//Everything I need is right here
for(int x = 0; x < 10; x++){}
In probability result, you can drop the if and the new variable. Just do it like this
private static bool probabilityResult(double percent)
{
if (percent <= 0 || percent > 1)
throw new ArgumentOutOfRangeException("Probability must be between 0 and 1.");
return rand.NextDouble() <= percent;
}
You can apply the same logic to several of your other functions. No need to save data to temp var if you're going to return it immediately
As far as actual performance goes, instead of using a multidimensional array ([,]), use a jagged array([][]). Due to compiler optimizations jagged arrays often perform better.
-
\$\begingroup\$ Thank you very, very much for the awesome feedback dude. I will try to integrate your suggestions. I'll have to read up on jagged arrays. Anyway, thanks so much and have an awesome day! :) \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月19日 18:07:10 +00:00Commented Apr 19, 2017 at 18:07
-
\$\begingroup\$ I wanted to update you on the situation Zebraman. Just to let you know I replaced the arrays with jagged arrays and the code takes about 3 seconds longer to execute. I suspect it's because of the needed initialization of the jagged arrays in the beginning but I'm not quite sure. \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月19日 19:44:51 +00:00Commented Apr 19, 2017 at 19:44
DRY
for (int y = 0; y < bufferHeight; y++) { for (int x = 0; x < bufferWidth; x++) { } }
If you happen to write the same code more then once it usually means something needs to be encapsulated.
As far as loops are concerned you have two choices:
- convenience first - go with LINQ and helper strucutres like
Point
- performance first - go with loops and delegates
I prefer the first option and switch do the second one only if something really seems to run slow and I notice and measured it.
So how would it look like if you ware lazy and wanted to get the job done quickly? You would create a map point generator:
public static class Map
{
public static IEnumerable<Point> GeneratePoints(int width, int height)
{
for (int y = 0; y < height; y++)
for (int x = 0; x < width; x++)
yield return new Point(x, y);
}
}
that you can reuse everywhere you need the coordinates:
foreach(var point in Map.GeneratePointS(bufferWidth, bufferHeight))
{
...
}
Assuming the spaceAvailable
method works with Point
you could greatly shorten the compareDistancesAndSetEachPoint
to soemthing like this:
var availablePoints = Map.GeneratePoints(bufferWidth, bufferHeight).Where(spaceAvailable);
foreach (var point in availablePoints)
{
...
}
In the second case you would use a delegate:
public static void ForEachPoint(int width, int height, Action<int, int, int> action)
{
var i = 0;
for (int y = 0; y < height; y++)
for (int x = 0; x < width; x++)
action(x, y, i++);
}
so the processing would now look like this
Map.ForEachPoint(bufferWidth, bufferHeight), (x, y, i) =>
{
...
});
where for each point the action
is executed however its signature Action<int, int, int>
isn't easy to understand and actually needs commenting. You can avoid this by creating a custom delegate:
public delegate void ProcessPointCallback(int x, int y, int index);
you would then change the signature of the ForEachPoint
method to
public static void ForEachPoint(int width, int height, ProcessPointCallback processPoint)
and the rest remains the same.
Other suggestions
private static bool spaceAvailable(int x, int y) { if (nodePositionValue[x, y] == 0) return true; else return false; }
You don't need an if for something like this. It's better to just write:
private static bool spaceAvailable(int x, int y)
{
return nodePositionValue[x, y] == 0;
}
while (true) { if (distance <= k * distanceInterval) { ... } k++; }
You should try to avoid nesting and actually use the while()
as an if
. This means you should put the condition for the loop where it belongs (and adjusting it of course):
while (distance > k * distanceInterval)
{
...
}
-
\$\begingroup\$ Wow. Such an informative review. I am really thankful for the points you made as I too was feeling that I shouldn't be repeating myself so much. The more object-oriented approach is preferable and I understand that because my code becomes really rigid and that's always a bad thing. The problem with me is that I seem to have this urge to always just do something. I haven't even begun to grasp the syntax and possibilities of C# and OOP but I just wanted to see what I could pull off with really bare bones basic knowledge. I have yet to learn all about delegates and the like and I will do so. \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月21日 19:36:31 +00:00Commented Apr 21, 2017 at 19:36
-
\$\begingroup\$ Sadly I still have important exams at the end of my last year in high school which I have to tent to first and I will for sure dedicate my entire summer to learn as much of the theory behind the language as I can. And again... thank you very much for dedicating your time for this review! :) \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月21日 19:39:13 +00:00Commented Apr 21, 2017 at 19:39
Ideally, you should not orchestrate the entire lifecycle of your application from Program class, but I left it...
There are also lots of things I was unable to understand, and I left them aside [consider as a "homework" for you :)].
I am using a few classes since I believe you wanted to see a more object oriented approach.
Notice that the code is still neither super clean (OOP-wise), nor super efficient, but gives you the basic direction (@Zebraman has a great comment on the underlying data structure you use). Let me know if you want anything to be explained in detail.
class Program
{
private static readonly int bufferWidth = 237;
private static readonly int bufferHeight = 90;
private static readonly double nodeSpawnProbability = 0.007; //Initial node spawn probability in each point of buffer array. (The higher this number is the more peaks on the map.)
private static readonly double intervalFraction = 0.03; //Number must be between 0 and 1. (The lower this number - the higher the variability and sharpness of terrain.)
private static readonly double maxDistanceFromCentre = Math.Sqrt(Math.Pow(bufferWidth / 2, 2) + Math.Pow(bufferHeight / 2, 2));
private static readonly double distanceInterval = maxDistanceFromCentre * intervalFraction;
static void Main(string[] args)
{
Console.SetBufferSize(bufferWidth, bufferHeight);
Console.ForegroundColor = ConsoleColor.Green;
var stopwatch = new Stopwatch();
stopwatch.Start();
var grid = new GridFactory()
.CreateGrid(bufferWidth, bufferHeight, nodeSpawnProbability); // First.
// Variable is unused...
var nodeCount = grid.NodeCount; // Second.
// CompareAndSet(grid); // Third.
stopwatch.Stop();
WriteTerrain(grid); // Final.
Console.SetBufferSize(bufferWidth, bufferHeight + 2);
Console.WriteLine();
Console.Write("Operation completed in: " + stopwatch.Elapsed.ToString());
Console.ReadKey(); //Wait before quitting.
}
// private static void CompareAndSet(Grid grid)
// {
// // TODO
// }
private static void WriteTerrain(Grid gridToPlot)
{
foreach (var location in gridToPlot.AllLocations)
{
Console.SetCursorPosition(location.X, location.Y);
Console.Write(gridToPlot[location.X, location.Y]);
}
}
private static void UpdateGridAmplitudes(Grid grid, int x, int y, double distance, int threshold)
{
int k = 1;
while (true)
{
if (distance <= k * distanceInterval)
{
if (k + threshold <= 9)
grid[x, y].Amplitude = threshold + k;
else
grid[x, y].Amplitude = 9;
break;
}
k++;
}
}
}
Something that creates a Grid:
public class GridFactory
{
private static readonly Random random = new Random();
public Grid CreateGrid(int bufferWidth, int bufferHeight, double nodeSpawnProbability)
{
var grid = new Grid(bufferWidth, bufferHeight);
foreach (var location in grid.AllLocations)
{
if (TrueIfRandomWith(nodeSpawnProbability))
grid[location.X, location.Y].Amplitude = RandomAmplitude();
}
return grid;
}
private bool TrueIfRandomWith(double probability)
{
if (probability <= 0 || probability > 1)
throw new ArgumentOutOfRangeException("Probability must be between 0 and 1.");
return random.NextDouble() <= probability;
}
private int RandomAmplitude()
{
return random.Next(1, 4);
}
}
Grid that represents a map:
public class Grid
{
private readonly int _bufferWidth;
private readonly int _bufferHeight;
private readonly GridCell[,] _matrix;
public Grid(int bufferWidth, int bufferHeight)
{
_bufferWidth = bufferWidth;
_bufferHeight = bufferHeight;
_matrix = new GridCell[_bufferWidth, _bufferHeight];
foreach (var location in AllLocations)
_matrix[location.X, location.Y] = new GridCell();
}
public GridCell this[int x, int y]
{
get
{
return _matrix[x, y];
}
set
{
_matrix[x, y] = value;
}
}
public IEnumerable<GridLocation> AllLocations
{
get
{
var xs = Enumerable.Range(0, _bufferWidth);
var ys = Enumerable.Range(0, _bufferHeight);
var allGridCoordinates = xs.SelectMany(x => ys.Select(y => new GridLocation { X = x, Y = y }));
return allGridCoordinates;
}
}
public int NodeCount
{
get
{
return AllLocations
.Count(location => this[location.X, location.Y].IsAvailable == false);
}
}
}
Grid location ("coordinate")
public class GridLocation
{
public int X { get; set; }
public int Y { get; set; }
}
Grid cell:
public class GridCell
{
public bool IsAvailable { get; set; } = true;
public int Amplitude { get; set; } = 0;
}
Update 1
There tons and tons of materials that explain what are the strengths (and the weaknesses) of the OOP. You can start from here.
Here are a few ideas about the application of OOP principles.
In OOP-ed code, the data and the related processing functions (methods) are encapsulated into small entities (classes). The encapsulation has various positive effects, and a few negative effects.
You may see that with the rewritten code we can change Grid
, GridLocation
, and GridCell
more independently. For example, if we need to have some extra information about each location on the map, we may add some fields to GridCell
class rather than creating a whole new int[,] extraDataMap
which is by the way completely disconnected from the space
and other maps...
It's really hard to explain the benefits of OOP in one example, but I think more will come as you learn through reading and practicing.
-
\$\begingroup\$ Please try to reason why you consider this to be a better solution than the OP's own solution. Or what kind of process lead to this solution. Remember that this is Code Review, not simply Code Rewrite. \$\endgroup\$holroy– holroy2017年04月19日 18:35:41 +00:00Commented Apr 19, 2017 at 18:35
-
\$\begingroup\$ This is proficiently tied together and it will definitely help me integrate OOP better in the future. I am very thankful for the effort you put in to this. I'll have more time to study this tomorrow but I have one initial question - What is the underscore's purpose when naming a variable or method alike? \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月19日 18:37:22 +00:00Commented Apr 19, 2017 at 18:37
-
\$\begingroup\$ @SimeonLaplev I will update the post with some references to OOP-related materials and discussions. The underscore is sometimes used to avoid name collision (e.g.
bufferWidth = bufferWidth
will reassign a parameter value to itself; we can either dothis.bufferWidth = bufferWidth;
or just rename the field we want to assign the value to). Many people do not likeprivate _fieldName
naming convention, but it's a matter of preference. I am neither encouraging nor discouraging from using such convention. \$\endgroup\$Igor Soloydenko– Igor Soloydenko2017年04月19日 18:41:36 +00:00Commented Apr 19, 2017 at 18:41 -
\$\begingroup\$ Aha... I see. So the naming convention is rather trivial as I understand. Ok. Thanks for the heads up and I will keep that in mind from now on. \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月19日 18:44:53 +00:00Commented Apr 19, 2017 at 18:44
The thing that screams "BEGINNER!" is the fact that you've placed everything in the Program class. Igor's opening remark touches upon this, but he doesn't do anything beyond giving it a mention. My remarks below may be used in conjunction with all the other answers.
As written, your code is extremely rigid. What if you ever need a different height or width? Or node probability or interval fraction? Those 4 properties seem to define a very specific grid. This is achieved by creating a class with an appropriate name, perhaps Grid or GridMap or something else as long as it is a meaningful, clear name.
You have 4 main properties (previously mentioned) that help describe the uniqueness of one grid over another. At a minimum, you would have a class defined as something like:
public class GridMap
{
public int Width { get; }
public int Height { get; }
public double NodeProbability { get; }
public double IntervalFraction { get; }
public GridMap(int bufferWidth, int bufferHeight, double nodeProbability, double intervalFraction)
{
//I leave it as homework that you validate the inputs
Width = bufferWidth;
Height = bufferHeight;
NodeProbability = nodeProbability;
IntervalFraction = intervalFraction;
}
}
That is our launching point. Let's build from there. We will add a few more constructors.
You have a few spots where you use Magic Numbers (example: see generateNodeAmplitude), but you also have spots where you define some static readonly fields. As these are value type fields, you could make these constants.
public class GridMap
{
public int Width { get; }
public int Height { get; }
public double NodeProbability { get; }
public double IntervalFraction { get; }
public const int DefaultWidth = 237;
public const int DefaultHeight = 90;
public const double DefaultNodeProbability = 0.007;
public const double DefaultIntervalFraction = 0.03;
public GridMap(int bufferWidth, int bufferHeight, double nodeProbability, double intervalFraction)
{
//I leave it as homework that you validate the inputs
Width = bufferWidth;
Height = bufferHeight;
NodeProbability = nodeProbability;
IntervalFraction = intervalFraction;
}
public GridMap(int bufferWidth, int bufferHeight) : this(bufferWidth, bufferHeight, DefaultNodeProbability, DefaultIntervalFraction)
{ }
public static GridMap DefaultGrid()
{
return new GridMap(DefaultWidth, DefaultHeight, DefaultNodeProbability, DefaultIntervalFraction);
}
}
Now you've opened up a world of possibilities for a much larger combinations of grids! Of course, many of your static methods in the Program class need to be moved into this new class, and no longer be static. That's left as homework for you.
-
\$\begingroup\$ I value the criticism about the rigidity of my code and will work upon my methods in the future. I am truly a beginner and since I don't have the needed experience I don't think about convenience when I'm writing the program I just think about the next step and that's all I have in my mind. Those logical conjunctions in my head I translate to code in basic 'if' statements and 'loops' without perspective, one after another. Fortunately, I recognize this and I assure you I will try my best in the future to do it correctly. At the moment school is a priority though. Thanks again, dude! \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月21日 19:47:03 +00:00Commented Apr 21, 2017 at 19:47
I have revised my program today and used some of the tips I got from the answers on this post. I went towards a more object-oriented approach and completely rewrote the program. I saw a MASSIVE performance increase. From 1:05 minutes to about 11 seconds! I have used 3 class files total. (counting the initial Program.cs class file) I will now post the code for the revised program here for future reference.
Program.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Diagnostics;
using System.Threading.Tasks;
namespace testing_program
{
class Program : Map
{
private static readonly int bufferWidth = 237;
private static readonly int bufferHeight = 90;
private static readonly double intervalFraction = 0.02;
private static readonly double maxDistanceFromCentre = Math.Sqrt(Math.Pow(bufferWidth / 2, 2) + Math.Pow(bufferHeight / 2, 2));
private static readonly double distanceInterval = maxDistanceFromCentre * intervalFraction;
static void Main(string[] args)
{
Console.SetBufferSize(bufferWidth, bufferHeight);
Console.ForegroundColor = ConsoleColor.Green;
string elapsedTime = null;
Stopwatch sw = new Stopwatch();
sw.Start();
GeneratePoints(bufferWidth, bufferHeight); //GeneratePoints is a method from the Map class.
compareAndSetConnectedNodes();
setPointsAmplitude();
sw.Stop();
elapsedTime = sw.Elapsed.ToString();
writeTerrain();
Console.SetBufferSize(bufferWidth, bufferHeight + 2);
Console.WriteLine();
Console.Write("Operation completed in: " + elapsedTime);
Console.ReadKey();
}
private static void writeTerrain()
{
foreach (Point point in points)
{
Console.SetCursorPosition(point.x, point.y);
Console.Write(point.Amplitude);
}
}
private static void compareAndSetConnectedNodes()
{
foreach (Point point in points) //For reference - 'points' is a list of all points generated by the Map class (points is a member of the Map class)
{
if (!point.IsNode)
{
int n = 0;
double minimumDistance = n;
foreach (Point p in points)
{
if (p.IsNode)
{
if (n == 0)
{
minimumDistance = Math.Sqrt(Math.Pow(point.x - p.x, 2) + Math.Pow(point.y - p.y, 2));
point.DistanceToNode = minimumDistance;
}
else if (minimumDistance > Math.Sqrt(Math.Pow(point.x - p.x, 2) + Math.Pow(point.y - p.y, 2)))
{
minimumDistance = Math.Sqrt(Math.Pow(point.x - p.x, 2) + Math.Pow(point.y - p.y, 2));
point.DistanceToNode = minimumDistance;
}
n++;
}
}
}
}
}
private static void setPointsAmplitude()
{
foreach (Point point in points)
{
if (!point.IsNode)
{
int k = 1;
while (point.DistanceToNode > k * distanceInterval)
{
k++;
}
//---------------------------------------COSMETIC 'IF' STATEMENT----------------------------------------- (Should be removed when using the data.)
if (k + point.ConnectedNodeAmplitude <= 9)
point.Amplitude = point.ConnectedNodeAmplitude + k;
else
point.Amplitude = 9;
//-------------------------------------------------------------------------------------------------------
}
}
}
}
}
Map.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
namespace testing_program
{
public class Map
{
protected static List<Point> points = new List<Point>();
public static void GeneratePoints(int width, int height)
{
for (int y = 0; y < height; y++)
for (int x = 0; x < width; x++)
points.Add(new Point(x, y));
}
}
}
Point.cs
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
namespace testing_program
{
public class Point
{
private static double nodeProbability = 0.01;
private static int lowestNodeValue = 3;
private bool _isNode;
private int _x;
private int _y;
private double distanceToNode = 0;
private int amplitude = 0;
private int connectedNodeAmplitude;
private static Random randy = new Random(); //randy is a cool name for a Random() object :D
public Point(int x, int y)
{
_x = x;
_y = y;
if (isNode(nodeProbability))
{
_isNode = true;
amplitude = randy.Next(1, lowestNodeValue + 1);
}
}
private static bool isNode(double nodeProbability)
{
return randy.NextDouble() <= nodeProbability;
}
//Property declarations---------------------------------------------------
public int x
{
get { return _x; }
}
public int y
{
get { return _y; }
}
public double NodeProbability
{
get { return nodeProbability; }
set { if (value > 0 && value <= 1) nodeProbability = value; }
}
public int LowestNodeValue
{
get { return lowestNodeValue; }
set { if (value > 0) lowestNodeValue = value; }
}
public int ConnectedNodeAmplitude
{
get { return connectedNodeAmplitude; }
set { if (value > 0) connectedNodeAmplitude = value; }
}
public int Amplitude
{
get { return amplitude; }
set { if (value > 0) amplitude = value; }
}
public double DistanceToNode
{
get { return distanceToNode; }
set { distanceToNode = value; }
}
public bool IsNode
{
get { return _isNode; }
}
//------------------------------------------------------------------------
}
}
NOTE: I am sorry if the code isn't absolutely touched up or is lacking comments at most places. This is all I had time to do this Sunday. Thank you to EVERYONE that participated in this thread and helped me view my mistakes and take on a better perspective on the situation. Really. You all rock!
-
\$\begingroup\$ Perfect! I'm glad you were able to improve your code ;-) \$\endgroup\$t3chb0t– t3chb0t2017年04月23日 17:14:18 +00:00Commented Apr 23, 2017 at 17:14
-
1\$\begingroup\$ Same. It felt awesome working with more classes and types and properties. I felt like a kid with Lego bricks. :D The performance increase honestly shook me. Who new getting rid of all those arrays would result in a performance boost. ;) \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月23日 17:16:31 +00:00Commented Apr 23, 2017 at 17:16
-
\$\begingroup\$ By the way, may I quickly ask if it was a good idea to use a 'List' to store the Points? If not, what else can I use in it's place? I'm not sure if it fits the purpose correctly. \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月23日 20:19:44 +00:00Commented Apr 23, 2017 at 20:19
-
\$\begingroup\$ I find a
List
is fine and most probably I'd use it myself... or an array but there are not many alternatives. One could say if the number of points is constant an array might be more appropriate. As a matter of fact if the list never changes currently I'd use theIImmutableArray
from theSystem.Collections.Immutable
NuGet package. \$\endgroup\$t3chb0t– t3chb0t2017年04月23日 20:25:24 +00:00Commented Apr 23, 2017 at 20:25 -
\$\begingroup\$ I see. Thanks for the response. I'll make sure to look up 'IImmutableArray'. But if I use an array how do I specify it's length? Just make it 'height * width' long by calling a constructor to set it? \$\endgroup\$Simeon Laplev– Simeon Laplev2017年04月23日 20:35:36 +00:00Commented Apr 23, 2017 at 20:35
Explore related questions
See similar questions with these tags.
space
ornodePosVal
is hard to infer even by looking at the usage examples. Here're a few broad suggestions: spell things out -- don't be lazy; find a meaningful name for a given entity; use expressive names for methods (void getNodeCount()
is confusing, we callget
but void doesn't return anything back)... \$\endgroup\$object-oriented
tag, and nothing suggests objects are used in here to model maps \$\endgroup\$