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

Compute correlation (3 types) of stock ticker

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

Problem

I'm developing a web application allowing a logged-in user to find most correlated stocks for selected stock ticker. The user chooses a ticker, period of time (10, 30, 60, 90) and type of correlation (KENDALLS, PEARSONS, SPEARMANS). Then on the server side, the 'magic' happens and the server responds to the client by providing a list of stocks sorted by correlation value.

My controller:

@RequestMapping(value = "/stock/correlation", method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE)
@RolesAllowed(AuthoritiesConstants.USER)
public TreeSet getCorrelationForSelectedTicker(@RequestParam(value = "correlation_type", required = true) CorrelationType correlationType, @RequestParam(value = "ticker", required = true) StockTicker ticker, @RequestParam(value = "period", required = true) int period) {
    return statisticService.computeCorrelation(ticker, period, correlationType);
}


Here is my service:

  • In the service, I have step and isComputing variables, because when the user starts computing correlation, every second I ask the server if a correlation is computing. I then calculate the percentage of progress and show it to the user via a progress bar. Is my way of informing user about progress correct, or should it be done better?



  • I wanted to use multithreading as I want to save time when computing a correlation. I made an inner class as you can see below. Is it correct?



```
@Service
public class CorrelationService {

@Inject
private StockDetailsRepository stockDetailsRepository;

private Correlation correlation;
private int step;
private boolean isComputing;

public TreeSet computeCorrelation(StockTicker correlationForTicker, int period, CorrelationType correlationType) {
correlation = getCorrelationImpl(correlationType, correlationForTicker, period, stockDetailsRepository);
ExecutorService executor = Executors.newFixedThreadPool(5);

for (StockTicker ticker : correlation.getTickersWit

Solution

Ok, so there's a lot that can be simplified here.

As Xean points out, your Executor is probably not buying you very much. Don't use it to solve a performance problem until you know you have one, and test thoroughly to validate you're getting a performance boost. If you really need to, make one thread that does all the correlation work. But again, make sure that the computation really is the bottleneck, because I don't think some simple math is going to cause any issues worth the complexity multithreading adds here.

The progress bar support is also probably not necessary. Correlation computations should be fast enough that the bar wouldn't even show up. Don't spend time on this until you know for sure you need it. Make sure you track computation time separately from network latency.

Also, your three classes are really tightly coupled. I took a stab at cleaning things up by breaking the coupling and removing the multithreading and progress bar indicators.

Service class:

@Service
public final class CorrelationService {

    public CorrelationService() {
        super();
    }

    public SortedSet computeCorrelation(
            final StockTicker correlationForTicker, 
            final int period, 
            final CorrelationType correlationType) {

        final EnumSet stocksToCorrelate = EnumSet.allOf(StockTicker.class);
        stocksToCorrelate.remove(correlationForTicker);

        final Correlator correlator = 
                new Correlator(correlationForTicker, period, correlationType.getStrategy());
        return correlator.correlateAll(stocksToCorrelate);
    }
}


Business logic:

public final class Correlator {

    @Inject
    private StockDetailsRepository stockDetailsRepository;

    private final double[] sourceClosePrices;
    private final int period;
    private final CorrelationStrategy correlationStrategy;

    public Correlator(
            final StockTicker ticker,
            final int period,
            final CorrelationStrategy correlationStrategy) {

        this.period = period;
        if (this.period  correlateAll(final Collection tickers) {
        final SortedSet correlations = new TreeSet();        
        for (final StockTicker ticker : tickers) {
            correlations.add(this.correlate(ticker));
        }
        return correlations;
    }

    private StockStatistic correlate(final StockTicker ticker) {
        final double[] targetClosePrices = this.getClosePrices(ticker);        
        final Double correlation = 
                correlationStrategy.correlate(sourceClosePrices, targetClosePrices);
        return new StockStatistic(correlation, ticker);
    }

    private double[] getClosePrices(final StockTicker ticker) {
        final List stockDetailsList = 
                this.stockDetailsRepository.findByStockTickerOrderByDateDesc(ticker, new PageRequest(1, this.period)).getContent();
        final double[] closePrices = new double[stockDetailsList.size()];

        int index = 0;
        for (final StockDetails stockDetails : stockDetailsList) {
            closePrices[index] = stockDetails.getClosePrice().doubleValue();
            index++;
        }

        return closePrices;
    }
}


Strategy definition:

public interface CorrelationStrategy {

    Double correlate(final double[] sourceClosePrices, final double[] targetClosePrices);
}


Abstract Strategy parent. Shared correlation implementation can go here:

public abstract class AbstractCorrelationStrategy implements CorrelationStrategy {

    @Override
    public Double correlate(
            final double[] sourceClosePrices,
            final double[] targetClosePrices) {

        if (sourceClosePrices.length != targetClosePrices.length) {
            return 0.0;
        }
        return doCorrelate(sourceClosePrices, targetClosePrices);
    }

    protected abstract Double doCorrelate(
            final double[] sourceClosePrices,
            final double[] targetClosePrices);

}

Code Snippets

@Service
public final class CorrelationService {

    public CorrelationService() {
        super();
    }

    public SortedSet<StockStatistic> computeCorrelation(
            final StockTicker correlationForTicker, 
            final int period, 
            final CorrelationType correlationType) {

        final EnumSet<StockTicker> stocksToCorrelate = EnumSet.allOf(StockTicker.class);
        stocksToCorrelate.remove(correlationForTicker);

        final Correlator correlator = 
                new Correlator(correlationForTicker, period, correlationType.getStrategy());
        return correlator.correlateAll(stocksToCorrelate);
    }
}
public final class Correlator {

    @Inject
    private StockDetailsRepository stockDetailsRepository;

    private final double[] sourceClosePrices;
    private final int period;
    private final CorrelationStrategy correlationStrategy;

    public Correlator(
            final StockTicker ticker,
            final int period,
            final CorrelationStrategy correlationStrategy) {

        this.period = period;
        if (this.period < 1) {
            throw new IllegalArgumentException("Period must be at least 1.");
        }

        this.correlationStrategy = correlationStrategy;
        Objects.requireNonNull(correlationStrategy);

        Objects.requireNonNull(ticker);
        this.sourceClosePrices = this.getClosePrices(ticker);
    }

    public SortedSet<StockStatistic> correlateAll(final Collection<StockTicker> tickers) {
        final SortedSet<StockStatistic> correlations = new TreeSet<StockStatistic>();        
        for (final StockTicker ticker : tickers) {
            correlations.add(this.correlate(ticker));
        }
        return correlations;
    }

    private StockStatistic correlate(final StockTicker ticker) {
        final double[] targetClosePrices = this.getClosePrices(ticker);        
        final Double correlation = 
                correlationStrategy.correlate(sourceClosePrices, targetClosePrices);
        return new StockStatistic(correlation, ticker);
    }

    private double[] getClosePrices(final StockTicker ticker) {
        final List<StockDetails> stockDetailsList = 
                this.stockDetailsRepository.findByStockTickerOrderByDateDesc(ticker, new PageRequest(1, this.period)).getContent();
        final double[] closePrices = new double[stockDetailsList.size()];

        int index = 0;
        for (final StockDetails stockDetails : stockDetailsList) {
            closePrices[index] = stockDetails.getClosePrice().doubleValue();
            index++;
        }

        return closePrices;
    }
}
public interface CorrelationStrategy {

    Double correlate(final double[] sourceClosePrices, final double[] targetClosePrices);
}
public abstract class AbstractCorrelationStrategy implements CorrelationStrategy {

    @Override
    public Double correlate(
            final double[] sourceClosePrices,
            final double[] targetClosePrices) {

        if (sourceClosePrices.length != targetClosePrices.length) {
            return 0.0;
        }
        return doCorrelate(sourceClosePrices, targetClosePrices);
    }

    protected abstract Double doCorrelate(
            final double[] sourceClosePrices,
            final double[] targetClosePrices);

}

Context

StackExchange Code Review Q#101492, answer score: 2

Revisions (0)

No revisions yet.