Generador de mapa de altura básica -- # campo con performance campo con algorithm campo con object-oriented camp codereview Relacionados El problema

Basic height map generator


6
vote

problema

Español

He hecho este generador de mapas de altura relativamente básico solo para ver si podía sacarlo porque todavía soy un novato cuando se trata de programar. Entonces, mi pregunta es: "¿Hay una forma bastante obvia que pueda optimizarlo?". Puse en esfuerzo para que el código sea tan limpio, ya que puedo escribirlo, así que cualquier entrada de una revisión rápida (si no tiene nada mejor que hacer, por supuesto), sería muy apreciado. Aquí está el código original :

  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;         }     } }   
Original en ingles

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;         }     } } 
           
         
         

Lista de respuestas

5
 
vote
vote
La mejor respuesta
 

Una pareja cosas rápidas:

Si está utilizando mientras que los bucles para iteraciones, solo use para los bucles.

  <cmath>1  

En el resultado de la probabilidad, puede eliminar la variable IF y la nueva variable. Solo hazlo así

  <cmath>2  

Puede aplicar la misma lógica a varias de sus otras funciones. No es necesario guardar los datos en Temp V si lo van a devolverlo inmediatamente

En cuanto al rendimiento real, en lugar de usar una matriz multidimensional ([,]), use una matriz jaggada ([] []). Debido a las optimizaciones de compilador, las matrices irregulares a menudo funcionan mejor.

 

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.

 
 
   
   
4
 
vote

seco

  for (int y = 0; y < bufferHeight; y++) {     for (int x = 0; x < bufferWidth; x++)     {     } }   

Si escribe el mismo código más, una vez que, por lo general, significa que se debe encapsular algo.

En lo que respecta a los bucles, tiene dos opciones:

  1. Conveniencia Primero, vaya con Linq y Ayudas Strucutres como Point
  2. el rendimiento primero, vaya con bucles y delegados

Prefiero la primera opción y el interruptor Haz el segundo solo si algo realmente parece funcionar lento y me doy cuenta y medido .

Entonces, ¿cómo se vería si se va perezoso y quería hacer el trabajo rápidamente? Crearías un generador de puntos de mapa:

  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);     } }   

que puede reutilizar en todas partes, necesita las coordenadas:

  foreach(var point in Map.GeneratePointS(bufferWidth, bufferHeight)) {     ... }   

Suponiendo el método spaceAvailable con Point Puede acortar en gran medida el compareDistancesAndSetEachPoint para siempre así:

  var availablePoints = Map.GeneratePoints(bufferWidth, bufferHeight).Where(spaceAvailable); foreach (var point in availablePoints) {     ... }   

En el segundo caso, usted utilizaría un delegado:

  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++); }   

Entonces, el procesamiento ahora se vería así

  Map.ForEachPoint(bufferWidth, bufferHeight), (x, y, i) => {     ... });   

Dónde para cada punto El Point0 se ejecuta sin embargo, su firma Point1 no es fácil de entender y, en realidad, necesita comentarios. Puede evitar esto creando un delegado personalizado:

  Point2  

Luego cambiaría la firma del método Point3 a

  Point4  

y el resto sigue siendo el mismo.


Otras sugerencias

  Point5  

No necesitas un Si por algo así. Es mejor escribir:

  Point6  

  Point7  

Debe intentar evitar la anidación y en realidad use el Point8 como un Point9 . Esto significa que debe colocar la condición para el bucle donde pertenece (y ajustándolo por supuesto):

  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);     } } 0  
 

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:

  1. convenience first - go with LINQ and helper strucutres like Point
  2. 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) {     ... } 
 
 
   
   
3
 
vote

Idealmente, no debe orquestar todo el ciclo de vida de su solicitud de la clase de programa, pero lo dejé ...

También hay muchas cosas que no pude entender, y los dejé a un lado [considera como una "tarea" para usted:).

Estoy usando algunas clases, ya que creo que querías ver un enfoque más orientado a objetos.

Observe que el código aún no es super limpio (OOP-WISE), ni súper eficiente, sino que le brinda la dirección básica (@zebrraman tiene un gran comentario sobre la estructura de datos subyacentes que usa). Déjame saber si quieres que se explique algo en detalle.

  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);     } } 1  

algo que crea una cuadrícula:

  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);     } } 2  

Grid que representa un mapa:

  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);     } } 3  

Ubicación de la cuadrícula ("coordenada")

  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);     } } 4  

Célula de cuadrícula:

  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);     } } 5  

Actualización 1

Hay toneladas y toneladas de materiales que explican cuáles son las fortalezas (y las debilidades) de la OOP. Puedes iniciar desde aquí .

Aquí hay algunas ideas sobre la aplicación de los principios de OOP.

En código OOP -ED, los datos y las funciones de procesamiento relacionadas (métodos) se encapsulan en entidades pequeñas (clases). La encapsulación tiene varios efectos positivos, y algunos efectos negativos.

Puede ver que con el código reescrito Podemos cambiar 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); } } 6 , 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); } } 7 y 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); } } 8 más independientemente. Por ejemplo, si necesitamos tener alguna información adicional sobre cada ubicación en el mapa, podemos agregar algunos campos a 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); } } 9 clase en lugar de crear un nuevo 99887766655443330 que es por La forma completamente desconectada del foreach(var point in Map.GeneratePointS(bufferWidth, bufferHeight)) { ... } 1 y otros mapas ...

Es muy difícil explicar los beneficios de OOP en un ejemplo, pero creo que más llegará a medida que aprenda a través de la lectura y la práctica.

 

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.

 
 
       
       
2
 
vote

¡La cosa que grita "principiante!" Es el hecho de que has colocado todo en la clase de programa. El comentario de apertura de Igor toca esto, pero él no hace nada más que darle una mención. Mis comentarios a continuación se pueden usar junto con todas las demás respuestas.

Como está escrito, su código es extremadamente rígido. ¿Qué pasa si alguna vez necesita una altura o ancho diferente? ¿O la probabilidad del nodo o la fracción de intervalo? Esas 4 propiedades parecen definir una cuadrícula muy específica. Esto se logra creando una clase con un nombre apropiado, tal vez cuadrícula o cuadrícula o algo más, siempre y cuando sea un nombre significativo y claro.

Tiene 4 propiedades principales (mencionadas anteriormente) que ayudan a describir la singularidad de una cuadrícula sobre otra. Como mínimo, tendrías una clase definida como algo así:

  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;        } }   

Ese es nuestro punto de lanzamiento. Vamos a construir desde allí. Añadiremos unos cuantos constructores más.

Tienes algunos lugares donde usa los números mágicos (ejemplo: vea a GeneratenoDeamplitud), pero también tiene manchas donde define algunos campos estáticos de lectura. Como estos son campos de tipo de valor, podría hacer estas constantes.

  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);     } }   

¡Ahora has abierto un mundo de posibilidades para combinaciones mucho más grandes de Grids! Por supuesto, muchos de sus métodos estáticos en la clase del programa deben trasladarse a esta nueva clase, y ya no ser estáticos. Eso se queda como tarea para ti.

 

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.

 
 
 
 
1
 
vote

He revisado mi programa hoy y usé algunos de los consejos que obtuve de las respuestas en esta publicación. Fui hacia un enfoque más orientado a objetos y reescribí completamente el programa. Vi un aumento de rendimiento masivo . De 1:05 minutos a unos 11 segundos! He usado 3 archivos de clase Total. (Contando el archivo de clase Program.CS inicial) Ahora publicaré el código para el programa revisado aquí para futuras referencias.

programa.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));         }     } }   

punto.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; }         }          //------------------------------------------------------------------------     } }   

nota : Lo siento si el código no se ha tocado absolutamente o que le faltan comentarios en la mayoría de los lugares. Esto es todo lo que tuve tiempo de hacer este domingo. Gracias a a todos que participaron en este hilo y me ayudaron a ver mis errores y asumir una mejor perspectiva sobre la situación. de verdad. ¡Todos ustedes!

 

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!

 
 
         
         

Relacionados problema

1  Pasando un objeto a la vista en CodeIgner  ( Passing an object to the view in codeigniter ) 
Estoy tratando de usar un objeto y persistirlo en la base de datos. He creado una clase en la carpeta de la biblioteca. <?php if (!defined('BASEPATH'))...

0  Creación de múltiples objetos del extracto de SQL Server  ( Creating multiple objects from sql server extract ) 
He creado una solución prototipo simplificada como una especie de prueba de concepto antes de comenzar un programa más grande. Aquí están los datos de prueb...

10  "Stardust" Simulador 2D Gravity - Seguimiento 1: Los planetas  ( Stardust 2d gravity simulator follow up 1 the planets ) 
Este es un seguimiento para el juego de gravedad del simulador 2D pregunta. Desde entonces, he desarrollado mis habilidades de JavaScript, y ahora esto...

8  Envoltura Libusb Library en C ++  ( Wrapping libusb library in c ) 
Quiero usar la biblioteca de Libusb en mi aplicación C ++. He creado clases que envuelven las funciones de Libusb. Puede ver que la API de Libusb se divide en...

5  Solicitud de Tkinter para administrar las tareas escolares  ( Tkinter application to manage school tasks ) 
Soy un principiante en la programación de la OOP y me pregunto cuál es la mejor manera de poner todas las ventanas y funciones de una aplicación de ventana en...

8  Árbol fractal orientado a objetos  ( Object oriented fractal tree ) 
Hice un árbol fractal orientado a objetos en JavaScript utilizando la biblioteca P5, consta de tres archivos: fraternal tree.js sucursal.js flower.js ...

6  Objetos de Java para un programa de sniffer de red  ( Java objects for a network sniffer program ) 
Estoy trabajando en un programa de sniffer de red, cuya estructura utiliza 3 clases principales: clase de marco - uno por marco monitoreado, sostiene la re...

3  General Pathfinder en Python  ( General pathfinder in python ) 
Recientemente trabajé en un proyecto donde tuve la siguiente configuración: $_{456}$6 Ninguno del Código es relevante, no tendrá que poder ejecutar nada...

1  Clase de juego, escrito usando enums  ( Playing card class written using enums ) 
Escribí una clase de tarjeta A Mientras vuelvas a esta publicación aquí: Pregunta previa Sé que ha pasado mucho tiempo, pero recientemente regresé al proyec...

5  ¿Podría mejorarse este sistema de profundidad para un juego?  ( Could this depth system for a game be improved ) 
Todavía soy nuevo en C ++ y no tengo una gran idea de mi codificación, así que estaría muy agradecido a cualquiera y todos los que le otorgan consejos. Ade...




© 2022 respuesta.top Reservados todos los derechos. Centro de preguntas y respuestas reservados todos los derechos