current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote -1 down vote favorite

I'm creating a class-based signal generator that, given a buy threshold, sell threshold and a list of list of indicators, creates a 'buy' of 'sell' signal on a given day if the indicator from that day is greater than the sell threshold.

But the day before it wasn't greater, so basically when the indicator crosses over the buy threshold, it gives a sell signal and a buy signal when the indicator crosses under the buy threshold (it's lower than the threshold on a day, but the day before it wasn't).

I am wondering if this working code could be cleaned up using zip.

class Directionalindicator_signals():

    def __init__(self, buy_threshold, sell_threshold, list_of_indicators):
        self.buy_threshold = buy_threshold
        self.sell_threshold = sell_threshold
        self.list_of_indicators = list_of_indicators

    def calculate(self):
        signals = ['']
        for i in range(1, len(self.list_of_indicators)):
            if self.list_of_indicators[i] > self.buy_threshold and self.list_of_indicators[i-1] <= self.buy_threshold:
                signals.append('Sell')
            elif self.list_of_indicators[i] < self.buy_threshold and self.list_of_indicators[i-1] >= self.buy_threshold:
                signals.append('Buy')
            else:
                signals.append('')
        return signals
share|improve this question

closed as off-topic by 200_success♦, kleinfreund, Gareth Rees, syb0rg, Uri Agassi Feb 19 '14 at 13:43

If this question can be reworded to fit the rules in the help center, please edit the question.
    
Yes, it can, and I've shown you almost exactly how to do so on SO: stackoverflow.com/a/21851228/3001761 –  jonrsharpe Feb 19 '14 at 9:26
5  
This question appears to be off-topic because it is primarily asking for code to be written (after a very similar question was already answered on Stack Overflow), rather than seeking a code review. –  200_success♦ Feb 19 '14 at 9:31

1 Answer 1

up vote 2 down vote

My first advice is stop writing classes if you don't need to : your class has only two methods an init and a proper method. This could probably be written as a simple function :

def calculate_directions(buy_threshold, sell_threshold, list_of_indicators):
    signals = ['']
    for i in range(1, len(list_of_indicators)):
        if list_of_indicators[i] > buy_threshold and list_of_indicators[i-1] <= buy_threshold:
            signals.append('Sell')
        elif list_of_indicators[i] < buy_threshold and list_of_indicators[i-1] >= buy_threshold:
            signals.append('Buy')
        else:
            signals.append('')
    return signals

Then it seems a bit more obvious than sell_threshold is not useful.

Also, as suggested by jonrsharpe's comment, you can use zip :

def calculate_directions(buy_threshold, list_of_indicators):
    signals = ['']
    for prev, curr in zip(list_of_indicators, list_of_indicators[1:]):
        if curr > buy_threshold and prev <= buy_threshold:
            signals.append('Sell')
        elif curr < buy_threshold and prev >= buy_threshold:
            signals.append('Buy')
        else:
            signals.append('')
    return signals

and this can be written using list comprehension :

def calculate_directions(buy_threshold, list_of_indicators):
    return ['']+[('Sell' if (curr > buy_threshold and prev <= buy_threshold) else ('Buy' if (curr < buy_threshold and prev >= buy_threshold) else '')) for prev, curr in zip(list_of_indicators, list_of_indicators[1:])]

In a real life situation, I'd recommend exctracting the logic in the list comprehension into a function.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.