Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

(You might also get better performance with jagged arrays jagged arrays.)

(You might also get better performance with jagged arrays.)

(You might also get better performance with jagged arrays.)

Add note about jagged arrays
Source Link
mjolka
  • 16.3k
  • 2
  • 30
  • 73

(You might also get better performance with jagged arrays .)

(You might also get better performance with jagged arrays .)

Source Link
mjolka
  • 16.3k
  • 2
  • 30
  • 73

The ref keyword

You don't need to use the ref keyword anywhere. Please see Method Parameters and Types on MSDN.

Naming

Please try to follow C# naming and capitalization conventions.

Globalization

Use the version of Convert.ToDouble that takes an IFormatProvider. The sample file you gave has numbers in the form 0,0421216848673947, which will be converted differently depending on the current thread's culture. On my machine, that string is converted to 421216848673947.

You will want something like this:

var provider = CultureInfo.GetCultureInfo("de-DE").NumberFormat;
...
bubbleFrame[b, a, c] = Convert.ToDouble(numbers[a], provider);

Repetition

There are 26 if statements, one for each surrounding element. That's a lot of repetition, and a lot of chances to make mistakes.

You can write this more succinctly by storing the offsets of the surrounding elements in an array.

private static readonly int[][] Offsets =
 (from i in Enumerable.Range(-1, 3)
 from j in Enumerable.Range(-1, 3)
 from k in Enumerable.Range(-1, 3)
 where !(i == 0 && j == 0 && k == 0)
 select new[] { i, j, k }).ToArray();
...
foreach (var offset in Offsets)
{
 var offsetA = a + offset[0];
 var offsetB = b + offset[1];
 var offsetC = c + offset[2];
 if (IsInRange(offsetA, sensor)
 && IsInRange(offsetB, sensor)
 && IsInRange(offsetC, frames)
 && bubbleFrame[offsetB, offsetA, offsetC] > threshold)
 {
 bubbleFrame[offsetB, offsetA, offsetC] = 0;
 bubbleCollection[offsetB, offsetA, offsetC] = bubbleCounter;
 }
}
...
private static bool IsInRange(int i, int max)
{
 return i >= 0 && i < max;
}

Algorithm

You can get a list of candidate bubbles in one pass through the matrix.

private static IEnumerable<Point> GetCandidateBubbles(int sensor, int frames, double threshold, double[,,] bubbleFrame)
{
 var candidates = new List<Point>();
 for (var c = 0; c < frames; c++)
 {
 for (var b = 0; b < sensor; b++)
 {
 for (var a = 0; a < sensor; a++)
 {
 var value = bubbleFrame[b, a, c];
 if (value >= threshold)
 {
 candidates.Add(new Point(a, b, c, value));
 }
 }
 }
 }
 
 return candidates;
}

Then process each candidate in order:

foreach (var candidate in GetCandidateBubbles(sensor, frames, threshold, bubbleFrame).OrderByDescending(point => point.Value))
{
 var a = candidate.A;
 var b = candidate.B;
 var c = candidate.C;
 var value = bubbleFrame[b, a, c];
 // We might have already popped the candidate.
 if (value < threshold)
 {
 continue;
 }
 if (!previousBubble.HasValue || previousBubble.Value != value)
 {
 bubbleCounter++;
 }
 ...

Counting bubbles in your 120MB sample file using this approach takes ~15s on my computer, ~13s of which is just reading the input.

lang-cs

AltStyle によって変換されたページ (->オリジナル) /