Simplify and stabilize trade evaluation #305

2021-01-03T01:38:45-08:00

scionaltera commented

2021-01-03 01:38:45 -08:00

(Migrated from github.com)

Added Exchange to OrderNotFoundException for better error message
Better encapsulation, some utility methods in Paper classes
Changed "force open condition" API to be easier to use
Rearranged evaluation in trade() to be easier to read
Better error handling in exitPosition()
Easier to debug NPEs in getVolumeForEntryPosition()
Easier to debug NPEs in getMinimumAmountForEntryPosition()
Renamed isTradeExpired() to isActivePositionExpired() to better reflect what it does
Trying out more modern way to open and close Mockito mocks in BaseTestCase

* Added Exchange to OrderNotFoundException for better error message * Better encapsulation, some utility methods in Paper classes * Changed "force open condition" API to be easier to use * Rearranged evaluation in trade() to be easier to read * Better error handling in exitPosition() * Easier to debug NPEs in getVolumeForEntryPosition() * Easier to debug NPEs in getMinimumAmountForEntryPosition() * Renamed isTradeExpired() to isActivePositionExpired() to better reflect what it does * Trying out more modern way to open and close Mockito mocks in BaseTestCase

scionaltera commented

2021-01-03 02:01:11 -08:00

(Migrated from github.com)

Getting rate limiting on this branch but using fixedExposure seems to prevent it. All the new concurrency calls getMaximumExposure() too often and that makes an API call to the exchange.

Getting rate limiting on this branch but using `fixedExposure` seems to prevent it. All the new concurrency calls `getMaximumExposure()` too often and that makes an API call to the exchange.

dazito (Migrated from github.com) reviewed

2021-01-05 07:09:30 -08:00

src/main/java/com/r307/arbitrader/service/cache/OrderVolumeCache.java

					
				@ -0,0 +1,86 @@

				package com.r307.arbitrader.service.cache;

I suggest to return optional instead of null. To avoid possible nasty NPE in the future

dazito (Migrated from github.com) reviewed

2021-01-05 07:13:05 -08:00

src/main/java/com/r307/arbitrader/service/cache/OrderVolumeCache.java

					
				@ -0,0 +68,4 @@

				        // add the new key to the cache

				        cache.put(computeCacheKey(exchange, orderId), volume);

				        keys.add(computeCacheKey(exchange, orderId));

What is the advantage of using a List here (keys) just to get the size of the cache? Isn't cache.size() enough? Or worst case we can do something like cache.keySet().size()

What is the advantage of using a `List` here (keys) just to get the size of the cache? Isn't `cache.size()` enough? Or worst case we can do something like `cache.keySet().size()`

dazito (Migrated from github.com) reviewed

2021-01-05 08:06:57 -08:00

src/main/java/com/r307/arbitrader/service/cache/OrderVolumeCache.java

					
				@ -0,0 +68,4 @@

				        // add the new key to the cache

				        cache.put(computeCacheKey(exchange, orderId), volume);

				        keys.add(computeCacheKey(exchange, orderId));

Oh, I guess it is because of the ordering?! Otherwise on the map we have no order guarantee...

scionaltera (Migrated from github.com) reviewed

2021-01-05 12:15:38 -08:00

src/main/java/com/r307/arbitrader/service/cache/OrderVolumeCache.java

					
				@ -0,0 +68,4 @@

				        // add the new key to the cache

				        cache.put(computeCacheKey(exchange, orderId), volume);

				        keys.add(computeCacheKey(exchange, orderId));

Correct. The List is to give an ordered way to identify the oldest entry so we can evict it from the cache. But we still need the Map too so we can return arbitrary things from the cache without iterating.

scionaltera (Migrated from github.com) reviewed

2021-01-05 12:16:36 -08:00

src/main/java/com/r307/arbitrader/service/cache/OrderVolumeCache.java

					
				@ -0,0 +1,86 @@

				package com.r307.arbitrader.service.cache;

Good idea. I'm converting all three caches to use Optional.

Good idea. I'm converting all three caches to use `Optional`.

Commenting is not possible because the repository is archived.

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

scion/arbitrader!305

				`@ -0,0 +1,86 @@`
				`package com.r307.arbitrader.service.cache;`

Rows
Columns