Skip to content

Conversation

@aksu11
Copy link

@aksu11 aksu11 commented Sep 17, 2025

#7
Changes

  • Improved calcCostByTicket: supports both position and deal tickets, aggregates costs correctly, swaps only once.
  • Tested with multiple positions and partial closes.

Copilot AI review requested due to automatic review settings September 17, 2025 18:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the calcCostByTicket function to handle both position and deal tickets while aggregating costs correctly and optimizing swap calculations. The changes address issues with cost calculation accuracy and function versatility.

  • Enhanced calcCostByTicket to support both position tickets and deal tickets with proper fallback logic
  • Improved cost aggregation by summing commission and fees from all related deals while avoiding double-counting swap
  • Added better error handling and informative debug messages

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Include/errordescription.mqh Added trailing newline for formatting consistency
Include/EAUtils.mqh Completely rewrote calcCostByTicket function with enhanced ticket handling and cost aggregation logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

// If we can, aggregate costs across all deals for the same position
if (pos_id != 0 && HistorySelectByPosition(pos_id)) {
int deals_total = HistoryDealsTotal();
pcomm = 0.0; pfee = 0.0; pswap = 0.0;
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

Variables pcomm, pfee, and pswap are being reused and reassigned in different contexts throughout the function. Consider using distinct variable names for different purposes (e.g., total_comm, total_fee, total_swap for aggregated values) to improve code clarity and reduce potential confusion.

Copilot uses AI. Check for mistakes.
double deal_fee = HistoryDealGetDouble(ticket, DEAL_FEE);
double deal_swap = 0.0; // optional
double tmp_swap;
if (HistoryDealGetDouble(ticket, DEAL_SWAP, tmp_swap)) deal_swap = tmp_swap;
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

The variable tmp_swap is only used once as an intermediary. You can simplify this by directly checking the return value: if (!HistoryDealGetDouble(ticket, DEAL_SWAP, deal_swap)) deal_swap = 0.0; This eliminates the unnecessary temporary variable.

Copilot uses AI. Check for mistakes.
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.

1 participant