v3.003-dev-new #744
v3.003-dev-new #744
Conversation
…LOSE/BUY composite signal
…ing to create new abstract instance of IndicatorBase class. Dict<long, IndicatorBase*> now have template specialization which will print an error and stop debugging if you try do deserialize such Dict.
* v3.002-dev-new: Account/AccountMt: Fixes zero division GHA: Compile: Support for workflow calls GHA: Compile: Skips clean-up by default GHA: Fixes compilation workflows Trade: Disables filling modes for MQL4 Workaround for closing order conditions after orders are loaded from active pool (GH-705) Trade: TradeParams: Adds max_spread Order: Fixes the current volume value when data in orequest is missing Order: Refresh order after modification Trade: Disables filling modes for MQL4 EA: Fixes recent issue with error during trade request Account/AccountMt: Fixes zero division
* v3.001-dev-new-stuff: Adds EA31337.vscode-mql-tools extension Fixes new lines [end-of-file-fixer] Disables wine package devcontainer: Adds node Adds wine to devcontainer Adds DavidAnson.vscode-markdownlint extension to devcontainer Adds initial .vscode/settings.json
…3-dev-new * origin/v3.001-dev-new-cgava: GHA: Compile: Support for workflow calls Trade: TradeSignalEnumMacro: Fixes includes to use local path Moves enum macros into the main file and fixes compilation errors Adds a wrapper file for TradeSignalEnumMacro Renames TradeEnumMacro.mq4 to TradeSignalEnumMacro.test.mq5 CGA(Trade): proposal for macor based computation of SIGNAL BUY/SELL CLOSE/BUY composite signal GHA: Compile: Skips clean-up by default
…EA31337-indicators-stats#1
* origin/v3.002-dev-new: Workaround for closing order conditions after orders are loaded from active pool (GH-705) Trade: TradeParams: Adds max_spread Order: Fixes the current volume value when data in orequest is missing Order: Refresh order after modification Trade: Disables filling modes for MQL4 Account/AccountMt: Fixes zero division Refs EA31337-classes/EA31337-indicators-other#13, EA31337-classes/EA31337-indicators-other#15. WIP. TDI-RT-Clone and Heiken_Ashi_Smoothed indicators made to work in MT5. GHA: Compile: Adds support for path input
* v3.002-dev-new: Strategy: Improves logic for TrendStrength() Strategy: Adds missing trend_threshold to constructor IndicatorData: Adds Set()
WalkthroughThis update refactors core framework components by replacing generic "Action" and "Condition" types with new, task-specific "TaskAction" and "TaskCondition" classes, enums, and structs. It updates includes, type names, and test code accordingly. Additional improvements include macro guards, pointer dereference fixes, new trade signal macros, and enhanced buffer management utilities. Changes
Sequence Diagram(s)sequenceDiagram
participant EA as EA
participant Task as Task
participant TaskAction as TaskAction
participant TaskCondition as TaskCondition
EA->>Task: Create TaskEntry(TaskActionEntry, TaskConditionEntry)
Task->>TaskAction: GetActions()
Task->>TaskCondition: GetConditions()
TaskAction->>TaskAction: Execute(TaskActionEntry)
TaskCondition->>TaskCondition: Test(TaskConditionEntry)
sequenceDiagram
participant Trade as Trade
participant Signal as TradeSignal
Trade->>Signal: Check signal flags using new macros
Signal-->>Trade: Return true/false for open/close buy/sell
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
SerializerConverter.mqh (1)
118-118: Inconsistent null pointer literal usageLine 111 uses
NULLwhile this line usesnullptr. Consider standardizing on one null pointer literal throughout the codebase.- SerializerConverter _converter(((C*)nullptr)PTR_DEREF Parse(data), 0); + SerializerConverter _converter(((C*)NULL)PTR_DEREF Parse(data), 0);Task/Task.struct.h (1)
75-78: Flag constants not renamed.While the struct and member types have been renamed to use the Task prefix, the flag constants (like ACTION_ENTRY_FLAG_IS_ACTIVE) have not been renamed to match the new naming convention.
Consider renaming these flag constants to include the TASK prefix for consistency:
- bool IsActive() { return HasFlag(ACTION_ENTRY_FLAG_IS_ACTIVE); } - bool IsDone() { return HasFlag(ACTION_ENTRY_FLAG_IS_DONE); } - bool IsFailed() { return HasFlag(ACTION_ENTRY_FLAG_IS_FAILED); } - bool IsValid() { return !HasFlag(ACTION_ENTRY_FLAG_IS_INVALID); } + bool IsActive() { return HasFlag(TASK_ACTION_ENTRY_FLAG_IS_ACTIVE); } + bool IsDone() { return HasFlag(TASK_ACTION_ENTRY_FLAG_IS_DONE); } + bool IsFailed() { return HasFlag(TASK_ACTION_ENTRY_FLAG_IS_FAILED); } + bool IsValid() { return !HasFlag(TASK_ACTION_ENTRY_FLAG_IS_INVALID); }Task/TaskAction.h (1)
377-377: Header guard not updated.The header guard still uses ACTION_MQH instead of TASK_ACTION_MQH, which doesn't match the new file naming convention.
Consider updating the header guard to match the new file name:
-#ifndef ACTION_MQH -#define ACTION_MQH +#ifndef TASK_ACTION_MQH +#define TASK_ACTION_MQH // Content of the file -#endif // ACTION_MQH +#endif // TASK_ACTION_MQHStd.h (1)
264-417: High macro duplication – explore variadic helpers
ACQUIRE_BUFFER1…ACQUIRE_BUFFER8and the matchingRELEASE_BUFFER*blocks are copy-pasted eight times. This is error-prone and hard to maintain (future changes must be done in sixteen places).A single variadic macro can do the job:
#define _FOR_EACH(MACRO, ...) EXPAND(_FOR_EACH_N(MACRO, __VA_ARGS__, _R8,_R7,_R6,_R5,_R4,_R3,_R2,_R1)(_FOR_EACH_ ## MACRO, __VA_ARGS__)) /* … generic machinery omitted for brevity … */ #define ACQUIRE_BUFFERS(...) \ SET_BUFFER_AS_SERIES_RELEASE_ENSURER_BEGIN(ARGC(__VA_ARGS__)); \ _FOR_EACH(SET_BUFFER_AS_SERIES_FOR_TARGET, __VA_ARGS__) #define RELEASE_BUFFERS(...) \ _FOR_EACH(SET_BUFFER_AS_SERIES_FOR_HOST, __VA_ARGS__) \ SET_BUFFER_AS_SERIES_RELEASE_ENSURER_END(ARGC(__VA_ARGS__))This removes 300+ lines and the risk of the two blocks diverging.
Task/TaskCondition.h (1)
100-128: Iterator copies large struct by valueInside
Test()each loop iteration copies a wholeTaskConditionEntry(_entry = iter.Value();).
The struct is already sizable (flags, timestamps, flexible arg array) — copying it repeatedly is unnecessary.- TaskConditionEntry _entry = iter.Value(); + TaskConditionEntry & _entry = iter.Value();Using a reference avoids extra allocations and keeps any flag changes (‘invalid’ etc.) in sync with the dictionary entry.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
.coderabbit.yamlis excluded by none and included by none.devcontainer/devcontainer.jsonis excluded by none and included by none.vscode/settings.jsonis excluded by none and included by none
📒 Files selected for processing (40)
.github/workflows/compile-cpp.yml(1 hunks).github/workflows/test.yml(0 hunks)Chart.mqh(1 hunks)Condition.struct.h(0 hunks)DictIteratorBase.mqh(1 hunks)DictObject.mqh(6 hunks)DictStruct.mqh(5 hunks)EA.mqh(1 hunks)EA.struct.h(2 hunks)Indicators/Indi_Drawer.mqh(3 hunks)Market.mqh(1 hunks)Math.define.h(1 hunks)Object.mqh(0 hunks)Order.mqh(1 hunks)Serializer.mqh(3 hunks)SerializerConverter.mqh(2 hunks)SerializerJson.mqh(1 hunks)Std.h(2 hunks)Strategy.mqh(1 hunks)Strategy.struct.h(1 hunks)Task/Task.enum.h(1 hunks)Task/Task.h(3 hunks)Task/Task.struct.h(2 hunks)Task/TaskAction.enum.h(3 hunks)Task/TaskAction.h(9 hunks)Task/TaskAction.struct.h(2 hunks)Task/TaskCondition.enum.h(4 hunks)Task/TaskCondition.h(5 hunks)Task/TaskCondition.struct.h(1 hunks)Task/tests/Task.test.mq4(1 hunks)Task/tests/Task.test.mq5(1 hunks)Task/tests/TaskAction.test.mq4(1 hunks)Task/tests/TaskAction.test.mq5(3 hunks)Task/tests/TaskCondition.test.mq4(1 hunks)Task/tests/TaskCondition.test.mq5(7 hunks)Trade.mqh(2 hunks)Trade/TradeSignal.struct.h(1 hunks)Trade/tests/TradeSignalEnumMacro.test.mq4(1 hunks)Trade/tests/TradeSignalEnumMacro.test.mq5(1 hunks)tests/CompileTest.mq5(3 hunks)
💤 Files with no reviewable changes (3)
- Object.mqh
- .github/workflows/test.yml
- Condition.struct.h
🧰 Additional context used
🧬 Code Graph Analysis (1)
Task/TaskAction.h (4)
Task/Task.h (13)
_entry(74-74)_entry(74-74)_entry(98-115)_entry(98-98)_cond(214-235)_cond(214-214)_cond(236-239)_cond(236-236)_result(82-90)_action(249-260)_action(249-249)_action(261-264)_action(261-261)Task/TaskAction.struct.h (6)
_obj(105-105)_obj(105-105)_flag(71-71)_flag(71-71)_flag(74-79)_flag(74-74)Task/Task.struct.h (3)
IsValid(78-78)IsValid(78-78)_action(47-47)Task/TaskCondition.struct.h (2)
IsValid(89-89)IsValid(89-89)
🔇 Additional comments (88)
SerializerJson.mqh (1)
30-30: LGTM: Added include for Serializer enumsThis addition aligns with the broader refactoring effort of modularizing enum and struct headers.
SerializerConverter.mqh (2)
33-33: LGTM: Added Object.mqh includeThis inclusion is necessary and consistent with the broader refactoring effort.
111-111: Improved pointer dereferencing safetyUsing the
PTR_DEREFmacro for pointer dereferencing is a safer practice than using direct dot notation on potentially null pointers..github/workflows/compile-cpp.yml (2)
39-41: LGTM: Added Emscripten toolchain supportAdding WebAssembly compilation capabilities extends platform compatibility.
45-48: LGTM: Dual compilation path implementationSplitting the compilation into separate emcc and g++ steps ensures code compatibility with both WebAssembly and native environments.
Math.define.h (1)
43-105: Added proper include guards for numeric limit macrosAdding conditional guards prevents redefinition warnings or errors when these macros might be defined elsewhere in the codebase or included libraries.
Strategy.mqh (1)
39-39: Include directive updated to new Task headerThe include has been changed to
"Task/Task.h", aligning with the reorganized Task subdirectory and.hextension. This keeps the file consistent with the new module structure.Strategy.struct.h (1)
37-37: Include directive updated to Task.struct.h pathThe include path for
Task.struct.hhas been updated to"Task/Task.struct.h", reflecting the relocated header in theTaskfolder and ensuring consistency with other Task module includes.Task/tests/TaskAction.test.mq4 (2)
24-24: Updated file header to reference TaskActionThe header comment now correctly states that this file tests the
TaskActionclass instead of the genericActionclass, matching the refactoring.
28-28: Include directive updated to TaskAction testThe include has been changed to
"TaskAction.test.mq5", aligning with the renamed test file forTaskAction.Order.mqh (1)
46-46: Include directive updated to TaskAction enumThe include of the old
"Action.enum.h"has been replaced with"Task/TaskAction.enum.h", aligning this file with the newTaskActionenumeration and directory structure.Task/tests/Task.test.mq4 (1)
28-28: Include directive updated to Task testThe include has been corrected to
"Task.test.mq5", matching the renamed Task test module.Task/tests/TaskCondition.test.mq4 (2)
24-24: Documentation updated to match renamed class.The test file comment now correctly reflects that it tests the
TaskConditionclass rather than the genericConditionclass, aligning with the broader refactoring in the PR.
28-28: Include path updated to match renamed file.The include has been properly updated to reference the renamed test file, ensuring consistency with the task-specific class naming scheme.
Task/Task.enum.h (2)
33-33: Updated enum documentation to reference TaskAction.Documentation now correctly references the new
TaskActionclass instead of the genericActionclass, maintaining consistency with the task-specific implementation.
40-40: Updated enum documentation to reference TaskCondition.Documentation now properly states that
ENUM_TASK_CONDITIONis for theTaskActionclass instead of the previously generic class, ensuring consistent documentation.Market.mqh (1)
37-37: Updated include path to reference task-specific condition enums.The include directive now points to the task-specific condition enum header, consistent with the PR's refactoring from generic to task-specific classes.
Chart.mqh (1)
48-48: Updated include path to reference task-specific condition enums.The include directive now points to the task-specific condition enum header, maintaining consistency with the PR's refactoring approach.
Trade.mqh (2)
41-42: Include paths updated to reflect module restructuring.The include paths have been updated to reference task-specific components from the new Task namespace. This change is part of a broader refactoring that replaces generic "Action" and "Condition" classes with more specific "TaskAction" and "TaskCondition" components.
1817-1817: Comment updated to match new class naming convention.The section header has been updated from "Actions" to "TaskActions" to maintain consistency with the renamed task-specific classes.
EA.struct.h (2)
35-35: Include path updated to reflect new directory structure.The include path for Task.struct.h has been updated to reflect its new location in the Task subdirectory. This change is part of the broader refactoring of task-related components.
228-228: Comment updated to use the new TaskAction naming convention.The comment for the flags member has been updated from "Action flags" to "TaskAction flags" to maintain consistency with the renamed task-specific classes.
EA.mqh (1)
47-49: Include paths updated to reference new Task module.The include paths have been updated to reference the reorganized task modules:
- Added Task/Task.h - Main task handling
- Added Task/TaskAction.enum.h - Task action enumerations
- Added Task/TaskCondition.enum.h - Task condition enumerations
This change is part of the refactoring that replaces generic "Action" and "Condition" with more specific "TaskAction" and "TaskCondition" components.
Task/tests/Task.test.mq5 (1)
31-37: Updated include paths to reflect new directory structure.The include paths have been updated to accommodate two changes:
- The test file is now located in a deeper directory (Task/tests), requiring additional "../" to reference framework files
- Action and Condition components have been renamed to TaskAction and TaskCondition
These changes ensure the test file properly references the restructured task-related components.
Trade/tests/TradeSignalEnumMacro.test.mq4 (1)
1-29: LGTM: Proper test wrapper file created.This new file correctly serves as an MQL4 wrapper to include and run tests defined in the corresponding MQL5 file, following the established pattern in the codebase for cross-platform testing.
tests/CompileTest.mq5 (1)
41-41: Updated includes to match the new task-related organization structure.The includes have been properly updated to reflect the reorganization of task-related headers into the
Tasksubdirectory with.hextensions, which is consistent with the broader refactoring effort.Also applies to: 70-70, 114-114
Trade/TradeSignal.struct.h (1)
53-58: Well-implemented trade signal evaluation macros.These new macros provide a concise way to verify valid, unfiltered trade signals by checking that the main signal bit is set while both filter and time filter bits are not set. The implementation correctly handles bitwise operations with appropriate operator precedence and explicit comparison against zero.
Task/TaskAction.enum.h (1)
24-24: Comment updates align with TaskAction refactoring.The comment changes appropriately reflect the renaming from generic "Action" to the more specific "TaskAction" context, maintaining consistency with the broader refactoring effort in the codebase.
Also applies to: 48-48, 59-59
DictObject.mqh (7)
145-145: Improved pointer dereferencing for better memory safetyThe pointer access syntax has been updated to use the
PTR_DEREFmacro, which provides a more consistent way to dereference pointers across different platforms and compiler environments.
147-147: Consistent pointer dereferencing syntaxUpdated pointer member access to use
PTR_DEREFfor pointer dereferencing, maintaining consistency with the rest of the codebase.
159-159: Standardized pointer member accessFollowing the pattern of using
PTR_DEREFfor accessing pointer members throughout the codebase.
174-174: Consistent pointer dereferencing implementationThe use of
PTR_DEREFmakes the pointer dereferencing explicit and consistent across all dictionary implementations.
189-189: Standardized pointer value comparisonUpdated to use
PTR_DEREFwhen accessing the value member of the slot pointer, maintaining consistent pointer dereferencing throughout the class.
268-269: Updated comment to match implementationComment updated to match the new
PTR_DEREFsyntax used in the code.
288-289: Updated comment for consistencyComment language updated to match the syntax changes using
PTR_DEREFfor pointer member access.Indicators/Indi_Drawer.mqh (3)
30-30: Updated import path for refactored TaskAction APIThe import path has been updated to use the new path for the refactored
TaskActionclass instead of the genericActionclass, aligning with the architectural changes in the framework.
109-109: Updated type to use task-specific action entryChanged from using the generic
ActionEntryto the more specificTaskActionEntryclass, which better represents its purpose in the task framework.
136-137: Consistent use of TaskActionEntryUpdated variable declaration to use the new
TaskActionEntrytype, maintaining consistency with the refactored task action system.DictStruct.mqh (6)
147-147: Improved pointer dereferencing with PTR_DEREFUpdated the pointer access to use the
PTR_DEREFmacro for more consistent dereferencing across different environments.
154-154: Standardized pointer member accessChanged direct pointer member access to use the
PTR_DEREFmacro, maintaining consistency with the codebase's pointer dereferencing approach.
169-169: Consistent pointer dereferencing in GetByKeyUpdated to use
PTR_DEREFwhen accessing the value member of the slot pointer.
186-186: Standardized pointer access in overloaded GetByKeyConsistently updated to use
PTR_DEREFfor pointer member access, following the pattern throughout the codebase.
202-202: Uniform pointer dereferencing in GetByPosUpdated to use the
PTR_DEREFmacro for accessing the value member of the slot pointer.
234-234: Consistent pointer value comparisonUpdated to use
PTR_DEREFwhen accessing the value member of the slot pointer for comparison.Task/Task.h (4)
33-34: Updated include paths to use relative pathsChanged the include paths to use relative paths (
../) which makes the code more maintainable when files are moved or reorganized.
37-38: Added new task-specific action and condition headersReplaced generic action and condition headers with the new task-specific versions, which better represent their purpose in the task framework.
101-103: Updated to use task-specific classesChanged from using generic
ConditionandActionclasses to the more specificTaskConditionandTaskActionclasses, improving code clarity and organization.
189-199: Improved control flow with clearer if-else structureReplaced a switch statement on boolean value with a more readable if-else structure, making the code easier to understand without changing its behavior.
Task/tests/TaskCondition.test.mq5 (5)
24-24: Updated file documentation to reflect TaskCondition focus.The file documentation has been correctly updated to reflect that this test file now focuses on the TaskCondition class rather than the Condition class.
31-34: Include paths correctly updated for the new directory structure.The include paths have been properly updated to match the new directory structure and file names, ensuring correct dependencies.
38-38: Updated dictionary to store TaskCondition objects.Dictionary type has been correctly changed to store TaskCondition objects instead of Condition objects.
81-82: Object type references updated to TaskCondition.All condition object instantiations have been correctly changed from Condition to TaskCondition, maintaining the same functionality while aligning with the refactoring effort.
Also applies to: 95-96, 109-111, 113-115, 157-159, 189-191
129-130: Updated indicator condition instantiations.Indicator condition tests were correctly updated to use TaskCondition objects while preserving the same test logic.
Also applies to: 132-133, 135-136, 138-139, 141-142, 144-145
Task/tests/TaskAction.test.mq5 (4)
24-24: Updated file documentation to reflect TaskAction focus.The file documentation has been correctly updated to reflect that this test file now focuses on the TaskAction class rather than the Action class.
34-37: Include paths correctly updated for the new directory structure.The include paths have been properly updated to match the new directory structure and file names, ensuring correct dependencies.
42-42: Updated dictionary to store TaskAction objects.Dictionary type has been correctly changed to store TaskAction objects instead of Action objects.
82-83: Object type references updated to TaskAction.Action object instantiations have been correctly changed to TaskAction, maintaining the same functionality while aligning with the refactoring effort.
Also applies to: 87-88
Serializer.mqh (3)
40-40: Added forward declaration for IndicatorBase class.Good practice to add the forward declaration for IndicatorBase before it's used in template specialization later in the code.
60-60: Constructor updated with default parameter value.The constructor now has a default value of 0 for the flags parameter, which makes instantiation more flexible.
382-395: Added specialized Pass template for IndicatorBase pointers.This specialized template overload properly handles serialization of IndicatorBase pointers while explicitly preventing deserialization with a clear error message. The use of nullptr for the error case aligns with modern C++ practices.
Trade/tests/TradeSignalEnumMacro.test.mq5 (4)
22-25: Added proper includes for test functionality.The test file correctly includes the necessary headers for test assertions and the trade signal structures being tested.
26-66: Comprehensive test cases for trade signal macros.The test function systematically verifies all combinations of signal flags to ensure that:
- Only the main signal flag alone returns true for each category
- Any combination including filter or time filter flags returns false
- The absence of the main flag always returns false
This is thorough test coverage for the macro behavior.
29-37: Well-structured trade signal test cases by signal type.The test cases are well-organized by signal type (CLOSE_BUY, CLOSE_SELL, OPEN_BUY, OPEN_SELL), making it easy to understand and maintain. Each section follows the same pattern, ensuring consistent test coverage.
Also applies to: 38-46, 48-56, 57-65
71-73: Simple but effective OnInit implementation.The OnInit function simply calls the testEnum function, which is a clean approach for a focused test file. This ensures all tests run during initialization.
Task/TaskAction.struct.h (5)
22-25: Good refactoring to use more specific TaskAction terminology.The file header comment now correctly specifies that this file contains TaskAction structs rather than generic Action structs, which aligns with the broader refactoring effort across the codebase.
33-44: Improved include organization and dependency management.The includes have been reordered and updated to use relative paths, adding specific enum headers earlier in the list. This better organizes dependencies and makes the file's requirements clearer.
46-55: Consistent renaming of struct and member comments.The renaming from ActionEntry to TaskActionEntry is consistent with the broader refactoring. Comments for members like flags, action_id, type, and args have been properly updated to reflect the TaskAction terminology.
57-65: Consistent constructor signatures update.All constructors have been renamed from ActionEntry to TaskActionEntry, maintaining consistency with the struct name change while preserving the same initialization logic.
67-69: Proper destructor renaming.The destructor has been properly renamed to match the new struct name.
Task/TaskCondition.enum.h (5)
22-25: Good refactoring to use more specific TaskCondition terminology.The file header comment now correctly specifies that this file contains TaskCondition's enums rather than generic Condition enums, aligning with the broader refactoring effort.
87-94: Consistent enum renaming.The enum has been renamed from ENUM_CONDITION_ENTRY_FLAGS to ENUM_TASK_CONDITION_ENTRY_FLAGS, while preserving the same flag values for backward compatibility.
97-102: Consistent enum type renaming.The enum has been renamed from ENUM_CONDITION_STATEMENT to ENUM_TASK_CONDITION_STATEMENT, maintaining the same enum values for backward compatibility.
105-120: Proper condition type renaming with updated comment.The enum has been renamed from ENUM_CONDITION_TYPE to ENUM_TASK_CONDITION_TYPE, and the comment for COND_TYPE_ACTION has been updated to reference TaskAction instead of Action.
167-176: Updated comment for clarity.The comment now correctly specifies "TaskAction conditions" instead of "Action conditions", maintaining consistency in the terminology.
Task/Task.struct.h (4)
33-36: Updated include paths to reflect new file names.The include paths now reference the renamed TaskAction.struct.h and TaskCondition.struct.h files, maintaining proper dependency management.
39-45: Consistent member type renaming.The types for action and cond members have been properly updated from ActionEntry and ConditionEntry to TaskActionEntry and TaskConditionEntry, along with their comments.
47-51: Updated constructor signature.The constructor now accepts TaskActionEntry and TaskConditionEntry types, and uses ENUM_TASK_CONDITION_TYPE instead of ENUM_CONDITION_TYPE.
82-85: Updated getter return types.The getter methods now properly return TaskActionEntry and TaskConditionEntry types, and ENUM_TASK_CONDITION_TYPE instead of ENUM_CONDITION_TYPE.
Task/TaskAction.h (9)
32-40: Updated class declarations and include paths.The forward class declaration has been properly renamed to TaskAction, and the include paths now reference the renamed TaskAction and TaskCondition files.
41-44: Class renamed to reflect task-specific purpose.The class has been appropriately renamed from Action to TaskAction, along with its comment, making its purpose more explicit.
52-53: Updated actions dictionary type.The actions dictionary now uses TaskActionEntry instead of ActionEntry, maintaining consistency with the struct renaming.
59-81: Updated constructor signatures and implementations.All constructors have been renamed to TaskAction and updated to create TaskActionEntry instances instead of ActionEntry instances.
95-98: Updated iterator types.The iterator types and value retrieval have been updated to work with TaskActionEntry instead of ActionEntry.
112-117: Updated Execute method signature and implementation.The Execute method now accepts a TaskActionEntry reference and casts the object to TaskAction instead of Action.
331-332: Updated method call to use TaskAction.The method call has been properly updated to use TaskAction::CheckCondition instead of Action::CheckCondition.
371-372: Updated method call to use TaskAction.The method call has been properly updated to use TaskAction::ExecuteAction instead of Action::ExecuteAction.
324-328:✅ Verification successful
Changed error handling behavior.
The method now returns
falseimmediately after logging an error, rather than continuing to the end of the method. This is a behavioral change that might affect error handling.Please verify that all calling code expects this immediate return behavior. Run the following script to find all calls to this method:
🏁 Script executed:
#!/bin/bash # Find all calls to CheckCondition to verify they handle the new immediate return behavior rg -A 3 $'CheckCondition\s*\(\s*ENUM_ACTION_CONDITION'Length of output: 505
No external callers affected by early return
A search for all invocations ofCheckCondition(ENUM_ACTION_CONDITION…)only returned its two definitions in Task/TaskAction.h (the overloaded wrapper and the main implementation). There are no other call sites, so the new immediate-falsereturn won’t break any upstream code.Task/TaskCondition.struct.h (1)
97-108: TODO placeholders block functionality
AddArg()andSetArgs()are currently empty stubs with@todocomments.
Any caller relying on argument storage will silently lose data.Please implement or at least assert/alert so that incorrect usage is caught early.
| #ifndef ULONG_MAX | ||
| #define ULONG_MAX std::numeric_limits<short>::max() | ||
| #endif |
There was a problem hiding this comment.
Fix incorrect type in ULONG_MAX definition
The ULONG_MAX macro is incorrectly using short type instead of unsigned long.
#ifndef ULONG_MAX
-#define ULONG_MAX std::numeric_limits<short>::max()
+#define ULONG_MAX std::numeric_limits<unsigned long>::max()
#endifThis will cause ULONG_MAX to return a much smaller value than expected (maximum value of short instead of unsigned long), potentially leading to buffer overflows or other memory safety issues.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #ifndef ULONG_MAX | |
| #define ULONG_MAX std::numeric_limits<short>::max() | |
| #endif | |
| #ifndef ULONG_MAX | |
| #define ULONG_MAX std::numeric_limits<unsigned long>::max() | |
| #endif |
🤖 Prompt for AI Agents
In Math.define.h around lines 87 to 89, the ULONG_MAX macro is incorrectly
defined using std::numeric_limits<short>::max(), which gives the maximum value
of a short instead of an unsigned long. To fix this, change the type in
std::numeric_limits from short to unsigned long so that ULONG_MAX correctly
represents the maximum value of an unsigned long.
| @@ -121,7 +121,7 @@ class DictIteratorBase { | |||
| bool IsLast() { | |||
| if (!IsValid()) return true; | |||
|
|
|||
| if (_dict PTR_DEREF GetMode() == DictModeUnknown || _dict.Size() == 0) { | |||
| if (_dict PTR_DEREF GetMode() == DictModeUnknown || _dict PTR_DEREF Size() == 0) { | |||
There was a problem hiding this comment.
Fixed pointer dereferencing in IsLast method.
The code now properly dereferences the _dict pointer before calling the Size() method, making it consistent with other pointer dereferencing patterns in the file and preventing potential runtime errors.
🤖 Prompt for AI Agents
In DictIteratorBase.mqh at line 124, the pointer _dict was not properly
dereferenced before calling the Size() method, which can cause runtime errors.
Fix this by ensuring _dict is dereferenced using PTR_DEREF before calling
Size(), matching the dereferencing style used with GetMode() and maintaining
consistency.
| #define SET_BUFFER_AS_SERIES_RELEASE_ENSURER_BEGIN(NUM_BUFFS) \ | ||
| AsSeriesReleaseEnsurer _as_series_release_ensurer(NUM_BUFFS); | ||
| #define SET_BUFFER_AS_SERIES_RELEASE_ENSURER_END(NUM_BUFFS) _as_series_release_ensurer.done(NUM_BUFFS); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Macro variable name will collide on multiple acquisitions
SET_BUFFER_AS_SERIES_RELEASE_ENSURER_BEGIN instantiates a variable named _as_series_release_ensurer without any attempt at uniquifying it.
If two ACQUIRE_BUFFERx macros are used in the same lexical scope, the second expansion redeclares the same variable → compilation error.
Consider incorporating __COUNTER__ (or __LINE__ as a fallback for MQL) to make the name unique per expansion.
🤖 Prompt for AI Agents
In Std.h around lines 306 to 309, the macros
SET_BUFFER_AS_SERIES_RELEASE_ENSURER_BEGIN and
SET_BUFFER_AS_SERIES_RELEASE_ENSURER_END declare a variable named
_as_series_release_ensurer without uniqueness, causing redeclaration errors if
used multiple times in the same scope. Modify the macros to append __COUNTER__
or __LINE__ to the variable name to ensure each expansion creates a uniquely
named variable, preventing collisions during compilation.
| // Ensures that we do RELEASE_BUFFERx after ACQUIRE_BUFFERx. | ||
| struct AsSeriesReleaseEnsurer { | ||
| bool released; | ||
| int num_buffs; | ||
| AsSeriesReleaseEnsurer(int _num_buffs) : released(false), num_buffs(_num_buffs) {} | ||
| void done(int _num_buffs) { | ||
| if (_num_buffs != num_buffs) { | ||
| Alert("You have acquired ", num_buffs, " buffers via ACQUIRE_BUFFER", num_buffs, | ||
| "(), but now trying to release with mismatched RELEASE_BUFFER", _num_buffs, "()!"); | ||
| DebugBreak(); | ||
| } | ||
|
|
||
| if (released) { | ||
| Alert("You have used RELEASE_BUFFER", num_buffs, "() again which is not required!"); | ||
| DebugBreak(); | ||
| } | ||
|
|
||
| released = true; | ||
| } | ||
| ~AsSeriesReleaseEnsurer() { | ||
| if (!released) { | ||
| Alert("You have used ACQUIRE_BUFFER", num_buffs, "() but didn't release buffer(s) via RELEASE_BUFFER", num_buffs, | ||
| "() before returning from the scope!"); | ||
| DebugBreak(); | ||
| } | ||
| } | ||
| }; |
There was a problem hiding this comment.
Alert / DebugBreak will break C++ compilation
The helper struct AsSeriesReleaseEnsurer unconditionally calls Alert() and DebugBreak() which only exist in the MQL runtime. When the same header is compiled in the C++ tool-chain ( e.g. unit-tests, host utilities ) the build will fail with unknown identifiers.
- Alert("You have acquired ", num_buffs, ...
- DebugBreak();
+ #ifdef __MQL__
+ Alert("You have acquired ", num_buffs, ...
+ DebugBreak();
+ #else
+ std::cerr << "[ACQUIRE/RELEASE mismatch] expected " << num_buffs
+ << " but got " << _num_buffs << std::endl;
+ std::abort();
+ #endifPlease wrap every Alert/DebugBreak call (also in the destructor) in an #ifdef __MQL__ guard or provide portable fall-backs so that dual compilation continues to work.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Std.h around lines 278 to 304, the calls to Alert() and DebugBreak() inside
the AsSeriesReleaseEnsurer struct cause compilation errors in the C++ tool-chain
because these functions only exist in the MQL runtime. To fix this, wrap every
Alert() and DebugBreak() call, including those in the destructor, with #ifdef
__MQL__ preprocessor guards to ensure they are only compiled in the MQL
environment. Alternatively, provide portable fallback implementations for these
functions when compiling in C++ to maintain dual compilation compatibility.
| DictStruct<short, TaskConditionEntry> conds; | ||
|
|
There was a problem hiding this comment.
Dictionary member should store values, not pointers
conds is declared as a concrete DictStruct<short, TaskConditionEntry> object.
However the copy-constructor assigns a pointer to it:
TaskCondition(TaskCondition &_cond) { // line 93
conds = _cond.GetConditions(); // <- pointer-to-object
}DictStruct ← DictStruct* is not a valid assignment and will fail to compile.
- conds = _cond.GetConditions();
+ conds = *(_cond.GetConditions());(or store the pointer instead).
🤖 Prompt for AI Agents
In Task/TaskCondition.h around lines 58-59 and 93, the member 'conds' is a
DictStruct object but the copy constructor assigns a pointer returned by
_cond.GetConditions() to it, causing a type mismatch. To fix this, modify the
copy constructor to assign the actual DictStruct object returned by
GetConditions(), not a pointer, or change 'conds' to store a pointer if pointer
semantics are intended. Ensure the assignment matches the declared type of
'conds'.
| // Constructors. | ||
| void TaskConditionEntry() : type(FINAL_CONDITION_TYPE_ENTRY), cond_id(WRONG_VALUE) { Init(); } | ||
| void TaskConditionEntry(long _cond_id, ENUM_TASK_CONDITION_TYPE _type) : type(_type), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(TaskConditionEntry &_ce) { this = _ce; } |
There was a problem hiding this comment.
Illegal assignment to this in copy-constructor
void TaskConditionEntry(TaskConditionEntry &_ce) { this = _ce; }this is a read-only pointer – assigning to it is undefined behaviour and disallowed in both C++ and MQL.
Copy each field or simply delegate to the assignment operator:
- TaskConditionEntry(TaskConditionEntry &_ce) { this = _ce; }
+ TaskConditionEntry(const TaskConditionEntry &_ce) { *this = _ce; }(also make the parameter const).
🤖 Prompt for AI Agents
In Task/TaskCondition.struct.h at line 58, the copy constructor attempts to
assign to the read-only pointer `this`, which is illegal. To fix this, change
the parameter to a const reference and copy each member field individually or
delegate to the assignment operator by calling `*this = _ce;` inside the
constructor body.
| void TaskConditionEntry() : type(FINAL_CONDITION_TYPE_ENTRY), cond_id(WRONG_VALUE) { Init(); } | ||
| void TaskConditionEntry(long _cond_id, ENUM_TASK_CONDITION_TYPE _type) : type(_type), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(TaskConditionEntry &_ce) { this = _ce; } | ||
| void TaskConditionEntry(ENUM_ACCOUNT_CONDITION _cond_id) : type(COND_TYPE_ACCOUNT), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_CHART_CONDITION _cond_id) : type(COND_TYPE_CHART), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_DATETIME_CONDITION _cond_id) : type(COND_TYPE_DATETIME), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_EA_CONDITION _cond_id) : type(COND_TYPE_EA), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_INDICATOR_CONDITION _cond_id) : type(COND_TYPE_INDICATOR), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_MARKET_CONDITION _cond_id) : type(COND_TYPE_MARKET), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_ORDER_CONDITION _cond_id) : type(COND_TYPE_ORDER), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_STRATEGY_CONDITION _cond_id) : type(COND_TYPE_STRATEGY), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_TASK_CONDITION _cond_id) : type(COND_TYPE_TASK), cond_id(_cond_id) { Init(); } | ||
| void TaskConditionEntry(ENUM_TRADE_CONDITION _cond_id) : type(COND_TYPE_TRADE), cond_id(_cond_id) { Init(); } | ||
| // Deconstructor. | ||
| void ~TaskConditionEntry() { |
There was a problem hiding this comment.
Constructors/destructor use invalid syntax
The struct declares its (copy-)constructors and destructor with a return type void, e.g.
void TaskConditionEntry() : type(... // line 56
void ~TaskConditionEntry() { // line 70In C++/MQL constructors and destructors must not specify a return type.
This will not compile.
- void TaskConditionEntry() : type(FINAL_CONDITION_TYPE_ENTRY), cond_id(WRONG_VALUE) { Init(); }
+ TaskConditionEntry() : type(FINAL_CONDITION_TYPE_ENTRY), cond_id(WRONG_VALUE) { Init(); }
- void ~TaskConditionEntry() {
+ ~TaskConditionEntry() {Repeat for every constructor overload.
🤖 Prompt for AI Agents
In Task/TaskCondition.struct.h between lines 56 and 70, remove the 'void' return
type from all constructor and destructor declarations, as constructors and
destructors in C++/MQL must not have a return type. Change lines like 'void
TaskConditionEntry()' to 'TaskConditionEntry()' and 'void ~TaskConditionEntry()'
to '~TaskConditionEntry()' for all constructor overloads and the destructor.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Tests
Chores