4
\$\begingroup\$

From the previous post, I have incorporated the nice answers made by Chris and Chip01.


Code


package io.github.coderodde.statistics.run;

import java.text.DecimalFormat;
import java.text.DecimalFormatSymbols;
import java.text.NumberFormat;
import java.util.Locale;

/**
 * This record class encapsulates the run statistics.
 * 
 * @author Rodion "rodde" Efremov
 */
public final record RunStatistics(long minimumDuration,
                                  long maximumDuration,
                                  double meanDuration,
                                  double medianDuration,
                                  double standardDeviation) {
    
    @Override
    public String toString() {
        // NumberFormat.format() is not thread-safe, so instantiate and call 
        // here:
        final NumberFormat nf = NumberFormat.getInstance(Locale.getDefault());
        final DecimalFormatSymbols symbols = new DecimalFormatSymbols();
        symbols.setDecimalSeparator('.');
        symbols.setGroupingSeparator(',');
        final DecimalFormat df = new DecimalFormat("0.###", symbols);
        
        return new StringBuilder("min = ")
          .append(nf.format(minimumDuration))
          .append(" ns, max = ")
          .append(nf.format(maximumDuration))
          .append(" ns, mean = ")
          .append(df.format(meanDuration))
          .append(" ns, median = ")
          .append(df.format(medianDuration))
          .append(" ns, sd = ")
          .append(df.format(standardDeviation))
          .append(" ns")
          .toString();
    }
}

package io.github.coderodde.statistics.run;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;

/**
 * This class provides methods for obtaining the running time statistics.
 * 
 * @author Rodion "rodde" Efremov
 */
public final class Runner {
    
    private static final int MINIMUM_ITERATIONS = 5;

    public static RunStatistics measure(final Runnable runnable, 
                                        final int iterations) {
        
        Objects.requireNonNull(runnable, "The input runnable is null");
        checkIterations(iterations);
        
        double meanDuration = 0.0;
        double medianDuration;
        double standardDeviation;
        final List<Long> durations = new ArrayList<>(iterations);
        
        for (int iteration = 0;
                 iteration < iterations;
                 iteration++) {
            
            final long duration = measure(runnable);
            meanDuration += duration;
            durations.add(duration);
        }
        
        Collections.sort(durations);
        
        final long minimumDuration = durations.getFirst();
        final long maximumDuration = durations.getLast();
        
        meanDuration /= iterations;
        medianDuration = computeMedianDuration(durations);
        standardDeviation = computeStandardDeviation(durations, meanDuration);
        
        return new RunStatistics(minimumDuration,
                                 maximumDuration,
                                 meanDuration, 
                                 medianDuration,
                                 standardDeviation);
    }
    
    public static RunStatistics measure(final List<Runnable> runnables) {
        Objects.requireNonNull(runnables, "The input runnables is null");
        
        if (runnables.isEmpty()) {
            throw new IllegalArgumentException("Nothing to measure");
        }
        
        double meanDuration = 0.0;
        double medianDuration;
        double standardDeviation;
        final List<Long> durations = new ArrayList<>(runnables.size());
        
        for (final Runnable runnable : runnables) {
            
            final long duration = measure(runnable);
            
            meanDuration += duration;
            durations.add(duration);
        }
        
        Collections.sort(durations);
        
        long minimumDuration = durations.getFirst();
        long maximumDuration = durations.getLast();
        
        meanDuration /= runnables.size();
        medianDuration = computeMedianDuration(durations);
        standardDeviation = computeStandardDeviation(durations, meanDuration);
        
        return new RunStatistics(minimumDuration,
                                 maximumDuration,
                                 meanDuration, 
                                 medianDuration,
                                 standardDeviation);
        
    }
    
    private static long measure(final Runnable runnable) {
        final long ta = System.nanoTime();
        runnable.run();
        final long tb = System.nanoTime();
        final long duration = tb - ta;
        return duration;
    }
    
    private static double computeMedianDuration(final List<Long> durations) {
        if (durations.size() % 2 == 1) {
            return durations.get(durations.size() / 2);
        } else {
            final int index1 = durations.size() / 2;
            final int index2 = index1 - 1;
            
            return (durations.get(index1) + 
                    durations.get(index2)) / 2.0;
        }
    }
    
    private static double computeStandardDeviation(final List<Long> durations,
                                                   final double meanDuration) {
        double sum = 0.0;
        
        for (final Long duration : durations) {
            sum += Math.pow(duration - meanDuration, 2.0);
        }
        
        return Math.sqrt(sum / durations.size());
    }
    
    private static void checkIterations(final int iterations) {
        if (iterations < MINIMUM_ITERATIONS) {
            final String exceptionMessage = 
                    String.format("Number of iterations (%d) is too small. " + 
                                  "Must be at least %d.", 
                                  iterations,
                                  MINIMUM_ITERATIONS);
            
            throw new IllegalArgumentException(exceptionMessage);
        }
    }
}

package io.github.coderodde.statistics.run.demo;

import io.github.coderodde.statistics.run.RunStatistics;
import io.github.coderodde.statistics.run.Runner;
import java.util.Random;

public class Demo {

    private static final Random RANDOM = new Random(13L);
    
    public static void main(String[] args) {
        final RunStatistics runStatistics = Runner.measure(() -> {
            try {
                Thread.sleep(RANDOM.nextInt(100));
            } catch (InterruptedException ex) {}
        }, 20);
        
        System.out.println(runStatistics);
    }
}

Typical output

min = 18,500 ns, max = 98,307,500 ns, mean = 48755625 ns, median = 56061450 ns, sd = 30235412.72 ns

Critique request

Please, tell me whether I am getting anywhere.

\$\endgroup\$
0

2 Answers 2

6
\$\begingroup\$

Nitpicking on style.

Meaningful but pointless names

It seems interesting to me that you give a nice meaningful variable name like iteration to a variable that you don't ever actually use in the loop body. This has you employing an interesting indentation style to avoid a long line.

        for (int iteration = 0;
                 iteration < iterations;
                 iteration++) {
            
            final long duration = measure(runnable);
            meanDuration += duration;
            durations.add(duration);
        }

In this scenario I don't think this gains us anything over the following.

        for (int i = 0; i < iterations; i++) {            
            final long duration = measure(runnable);
            meanDuration += duration;
            durations.add(duration);
        }

Order of arguments

It may just be weird, but reading this I found myself thinking, "where did that 20 come from?"

        final RunStatistics runStatistics = Runner.measure(() -> {
            try {
                Thread.sleep(RANDOM.nextInt(100));
            } catch (InterruptedException ex) {}
        }, 20);

Now, that 20 probably should not be a magic number, but rather a named constant.

However, I can't also help but think the simpler argument (the number of times to iterate) should come first.

Perhaps the following reads better?

        final RunStatistics runStatistics = Runner.measure(
            20,
            () -> {
                try {
                    Thread.sleep(RANDOM.nextInt(100));
                } catch (InterruptedException ex) {}
            }
        );
\$\endgroup\$
5
  • \$\begingroup\$ Personally, that feels slightly too clever. The for loop with i feels very idiomatic. \$\endgroup\$ Commented Nov 23 at 16:47
  • 1
    \$\begingroup\$ Robert Martin has a good explanation on why "i" is a good loop counter name despite it going against pretty much all naming lessons. The complexity of a variable name should be proportional to it's scope. The larger the scope the more information the variable name has to contain. The loop counter has a very small scope so the reader never has to scroll back to find what it means. \$\endgroup\$ Commented Nov 24 at 7:34
  • 1
    \$\begingroup\$ Contd... Interestingly enough, function names are the opposite. The larger the scope, the shorter the function name should be (which really means that widely used functions should only do simple things with no weird side effects, like "open" a file or "print" text to console). \$\endgroup\$ Commented Nov 24 at 7:34
  • 1
    \$\begingroup\$ @TorbenPutkonen: In addition, I would argue that identifier names are not English, they are Programmerese, and i is a well-established word in Programmerese with very rich and well-defined semantics, very much the same as the three letters "E", "m", and "c" are in physics ("E = mc²"). \$\endgroup\$ Commented Nov 26 at 18:53
  • \$\begingroup\$ Is i good? Bad? Weird? Doesn't really matter anymore. It's an idiom. \$\endgroup\$ Commented Nov 26 at 18:55
3
\$\begingroup\$

These are just a few quick addenda to any other comments you receive:

Why are you displaying times in nanoseconds? Is that likely to be useful to humans?

Why are you sometimes displaying times in ns with commas in them, and sometimes without? If numbers with thousands grouping are easier to read, why not use that format consistently?

Why are you mucking around with StringBuilder, NumberFormat, DecimalFormat, and DecimalFormatSymbols when Java has had printf, format, and formatted for a long time?

All of this adds up to it being easier to simply write:

double seconds(long ns) {
    return ns / 1_000_000_000.0;
}

// . . .

return "min = %.6f seconds, max = %.6f seconds, " +
    "mean = %.6f seconds, median = %.6f seconds, sd = %.6f seconds"
    .formatted(seconds(minimumDuration), seconds(maximumDuration),
        seconds(meanDuration), seconds(medianDuration),
        seconds(standardDeviation));

Wouldn't that be a lot simpler?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Yes, the unnecessary use of StringBuilder was my first reaction too... a simple format string is much more expressive in this case. \$\endgroup\$ Commented Nov 25 at 0:08

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.