adjust log format in LogTradesAnalysis Analyze #6
Conversation
2. adjust profit calculation in LogTradesAnalysis Analyze
2. add stop gain rule
There was a problem hiding this comment.
Hey @beaquant,
I think this looks ok. I have a couple concerns though. The name of the PR suggests one thing, but you've actually added a new rule as well as updating the log format of LogTradesAnalysis. I'd like to see the stop gain rule in a separate PR. Of the changes you listed, I've suggested a number of comments above. In addition, the tests are failing. You'll need to update those before we can move forward.
|
|
||
| profit := trade.ExitValue().Sub(trade.CostBasis()) | ||
| fmt.Fprintln(lta.Writer, fmt.Sprintf("%s - enter with %s %s (%s @ $%s)", | ||
| trade.EntranceOrder().ExecutionTime.Format(time.RFC822), trade.EntranceOrder().Side, trade.EntranceOrder().Security, trade.EntranceOrder().Amount, trade.EntranceOrder().Price)) |
|
|
||
| type stopLossRule struct { | ||
| Indicator | ||
| tolerance big.Decimal | ||
| } | ||
|
|
||
| type stopGainRule struct { |
There was a problem hiding this comment.
You actually don't need to define a duplicate struct here, since this one is private. I would redefine the above rule as
type stopRule struct {
Indicator
tolerace big.Decimal
}and then make an alias for both stopLoss rule and stopGain rule, eg.
type stopLossRule = stopRule
type stopGainRule = stopRule| @@ -20,8 +27,31 @@ func (slr stopLossRule) IsSatisfied(index int, record *TradingRecord) bool { | |||
| if !record.CurrentPosition().IsOpen() { | |||
| return false | |||
| } | |||
|
|
|||
| openPrice := record.CurrentPosition().CostBasis() | |||
| openPrice := record.CurrentPosition().EntranceOrder().Price | |||
There was a problem hiding this comment.
This looks like a breaking change. EntranceOrder().Price reflects the unit price you paid for a security, not the total value of the position. Even though I think what you're doing here isn't incorrect, it would be breaking change for anyone who was depending on this behavior.
rule_stop.go
Outdated
| loss := slr.Indicator.Calculate(index).Div(openPrice).Sub(big.ONE) | ||
| if record.currentPosition.IsShort() { |
There was a problem hiding this comment.
try to avoid using the private fields when you can. Here you should replace record.currentPosition with record.CurrentPosition()
| return loss.LTE(slr.tolerance) | ||
| } | ||
|
|
||
| // NewStopLossRule returns a new rule that is satisfied when the given loss tolerance (a percentage) is met or exceeded. |
There was a problem hiding this comment.
Docs are wrong here and on the line below
2. add stop gain rule
# Conflicts: # analysis.go
# Conflicts: # analysis.go # rule_stop.go
|
thanks, I'll double check it again |
use local time make more readability
add orderside string for trades analysis. because EntranceOrder may not be bought, but sold.