fix(sim) Add GetSaveCurrentEvent() to FairMCApplication#1605
fix(sim) Add GetSaveCurrentEvent() to FairMCApplication#1605karabowi wants to merge 2 commits intoFairRootGroup:devfrom
Conversation
📝 WalkthroughWalkthroughA new public getter method, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FairMCApplication
User->>FairMCApplication: Call SetSaveCurrentEvent(value)
FairMCApplication-->>FairMCApplication: Set fSaveCurrentEvent = value
User->>FairMCApplication: Call GetSaveCurrentEvent()
FairMCApplication-->>User: Return fSaveCurrentEvent
✨ 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fairroot/base/sim/FairMCApplication.h (1)
222-226: Fix typo and add const qualifier for better design.The getter implementation is correct and follows good naming conventions. However, there are two minor improvements:
- Fix the typo "goint" → "going" in the documentation
- Add
constqualifier since the method doesn't modify object stateApply this diff to improve the method:
- /** - * Returns the state of the flag deciding whether the current event - * is goint to be stored. - */ - Bool_t GetSaveCurrentEvent() { return fSaveCurrentEvent; } + /** + * Returns the state of the flag deciding whether the current event + * is going to be stored. + */ + Bool_t GetSaveCurrentEvent() const { return fSaveCurrentEvent; }The addition successfully addresses the PR objective of providing read access to the
fSaveCurrentEventflag and complements the existing setter nicely.
Allows the user to get the state of the fSaveCurrentEvent flag. Fixes issue FairRootGroup#1604.
Allows the user to get the state of the fMarkFill flag. Fixes issue FairRootGroup#1604.
|
Thanks! :-) Small bits…
Yes, you're "fixing" an issue. I'd call it resolving an issue. The issue IMHO is a feature request. P.S.: I hope the CI gets fixed at some point… |
ChristianTackeGSI
left a comment
ChristianTackeGSI
left a comment
There was a problem hiding this comment.
While you work on these, maybe move the initializers for fSaveCurrentEvent and fMarkFill to default member initializers?
| /** | ||
| * Returns the state of the flag deciding whether the current event | ||
| * is goint to be stored. | ||
| */ |
There was a problem hiding this comment.
Fix the typo: "going"
| * Returns the state of the flag deciding whether the current event | ||
| * is goint to be stored. | ||
| */ | ||
| bool GetSaveCurrentEvent() { return fSaveCurrentEvent; } |
There was a problem hiding this comment.
Mark this as const
| bool GetSaveCurrentEvent() { return fSaveCurrentEvent; } | |
| bool GetSaveCurrentEvent() const { return fSaveCurrentEvent; } |
| //** Mark/Unmark event to be filled into output. Default is TRUE. */ | ||
| void MarkFill(Bool_t flag) { fMarkFill = flag; } | ||
|
|
||
| //** Get the state of the flag deciding whether the current event is goint to be stored. */ |
There was a problem hiding this comment.
Please use consistant Doxygen formatting everywhere.
2 participants
Allows the user to get the state of the fSaveCurrentEvent flag.
Fixes issue #1604.
Checklist: