Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ascendex cancel order fix #4745

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Ascendex cancel order fix #4745

wants to merge 2 commits into from

Conversation

eeropu
Copy link

@eeropu eeropu commented Jul 24, 2023

Current implementation of cancelOrder method for Ascendex trade service does not work correctly. It removes all orders based on currency pair and ignores the orderId.

Current version of cancelOrder is renamed to cancelAllOrdersByCurrencyPair and the cancelOrder method has been changed to remove only the order with the given id.

cancelOrder method's functionality is now inline with other trade services' method with the same name.

@@ -32,6 +31,20 @@ public String placeLimitOrder(LimitOrder limitOrder) throws IOException {

@Override
public boolean cancelOrder(CancelOrderParams orderParams) throws IOException {
if (orderParams instanceof DefaultCancelOrderByInstrumentAndIdParams) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should type check by interface not their default implementation

please do orderParams instance of CancelOrderByIdParams && orderParams instanceof CancelOrderByInstrument

if both are needed

if only one is needed then use ||

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants