HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Tight loop, string manipulation and calculations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
tightloopcalculationsmanipulationandstring

Problem

I'm working on some precompilation operations for a world compiler. Currently to identify flags placed by the level designer I need recognize when a specific entity exists at specific coordinates from the flag's origin. I need to generate a list of offset coordinates from the origin(and include the origin) in string form. That is 7 coordinates. this operation is done for every flag in the world, so it is preferred to be efficient. It is not an operation that takes place during gameplay, so I don't have to worry too much.

My current implementation works correctly, and quickly, however I still would like to get a review on what I have written, and hear any advice to better this operation.

string origin = "0 0 0";
double[] originParts = origin.Split(' ').Select(s => double.Parse(s)).ToArray();
List origins = new List { origin };
for (int i = 0; i < 3; i++)
    for (int j = -48; j <= 48; j += 96)
        origins.Add(string.Format("{0} {1} {2}", 
            originParts[0] + (i == 0 ? j : 0),
            originParts[1] + (i == 1 ? j : 0), 
            originParts[2] + (i == 2 ? j : 0)));

Solution

This code is effectively calculating a set of offsets from the origin and adding them in turn to get the final co-ordinates. However, you're calculating the offsets each time for every flag and the question implies that this is done many times. Better to calculate them up-front, then iterate through them, applying the co-ordinates of the flag to each:

// Done once at initialization...
int[,] offsets = new int[6, 3] { { -48,   0,   0 }, 
                                 {  48,   0,   0 }, 
                                 {   0, -48,   0 }, 
                                 {   0,  48,   0 }, 
                                 {   0,   0, -48 },
                                 {   0,   0,  48 } }; ;

// Done for each flag in the game world...
string origin = "0 0 0";
double[] originParts = origin.Split(' ').Select(s => double.Parse(s)).ToArray();
List origins = new List { origin };

for(int idx = 0; idx < offsets.GetLength(0); idx++)
{
    origins.Add(String.Format("{0} {1} {2}", 
        originParts[0] + offsets[idx, 0],
        originParts[1] + offsets[idx, 1],
        originParts[2] + offsets[idx, 2]));
}


You could, of course, build the array of offsets programmatically instead of hardcoding them like I did.

This gives you slightly more lines of code, but it reduces the number of calculations done on each loop, and separates the calculation of the offsets from the calculation of the final co-ordinate, which may make future modifications simpler (e.g. if the designer wants more co-ordinates added to the list, or co-ordinates should be read from a configuration file, and so on).

Code Snippets

// Done once at initialization...
int[,] offsets = new int[6, 3] { { -48,   0,   0 }, 
                                 {  48,   0,   0 }, 
                                 {   0, -48,   0 }, 
                                 {   0,  48,   0 }, 
                                 {   0,   0, -48 },
                                 {   0,   0,  48 } }; ;

// Done for each flag in the game world...
string origin = "0 0 0";
double[] originParts = origin.Split(' ').Select(s => double.Parse(s)).ToArray();
List<string> origins = new List<string> { origin };

for(int idx = 0; idx < offsets.GetLength(0); idx++)
{
    origins.Add(String.Format("{0} {1} {2}", 
        originParts[0] + offsets[idx, 0],
        originParts[1] + offsets[idx, 1],
        originParts[2] + offsets[idx, 2]));
}

Context

StackExchange Code Review Q#51575, answer score: 3

Revisions (0)

No revisions yet.