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.

Sign up for GitHub

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

Jump to bottom

Fix #1495 #1496

Merged
merged 13 commits into from
May 26, 2024
Merged

Fix #1495 #1496

merged 13 commits into from
May 26, 2024

Commits on May 26, 2024

  1. test(Base): Add basic test coverage for FairVolumeList

    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    8755ae9 View commit details
    Browse the repository at this point in the history

  2. feat(Base): Eliminate extra allocation in FairVolumeList 

    * Explicitely delete copy ctor and assignment which were previously
private and unimplemented
* Disable unused streamer
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    5c3d912 View commit details
    Browse the repository at this point in the history

  3. feat(Base): Replace magic number with named constant

    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    82e7b5a View commit details
    Browse the repository at this point in the history

  4. feat(Base): Fix string arguments 

    * Passing a string ptr and then dereferencing without checking for
  `nullptr` clearly shows that the intention was to use a C++ reference
* Passing a string by value is making an unnecessary copy which can be
  avoided here.
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    dac92e1 View commit details
    Browse the repository at this point in the history

  5. feat(Base): Prevent memory leak with duplicates in FairVolumeList 

    The raw pointer arg is transferring ownership.
* In case the given object was not added to the list, it just leaked.
* A new overload with a unique_ptr arg avoids the leak and communicates the
  transfer of ownership.
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    7f0f7f1 View commit details
    Browse the repository at this point in the history

  6. fix(Base): Do not reimplement existing functions

    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    4a0e753 View commit details
    Browse the repository at this point in the history

  7. fix(Base): Simplify FairVolumeList further 

    * Avoid unnecessary expliciteness and track return value types with `auto`
* Use conditional operator for readability
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    4e4a3b6 View commit details
    Browse the repository at this point in the history

  8. feat(Base): Return added volume or nullptr

    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    f852585 View commit details
    Browse the repository at this point in the history

  9. fix(Base): Only search vList once 

    Fix FairRootGroup#1495
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    43962df View commit details
    Browse the repository at this point in the history

  10. refactor(Base): Rewrite loop with TRangeDynCast for readability 

    * Rename `aList` to `nodes`, see [S.2](https://github.com/FairRootGroup/FairRoot/blob/master/CONTRIBUTING.md#s2-prefer-variable-names-describing-what-they-contain)
* Apply [CPPCG::ES.6](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es6-declare-names-in-for-statement-initializers-and-conditions-to-limit-scope)
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    98aa8ba View commit details
    Browse the repository at this point in the history

  11. refactor(Base): FairModule::ProcessNodes further 

    * [CPPCG::ES.5 Keep scopes small](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es5-keep-scopes-small)
* [CPPCG::ES.11 Use `auto` to avoid redundant repetition of type
  names](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-auto)
* Rename `fNodes` to `parNodes` - the `f`-prefix is reserved for member
  variables
* Default to prefix increment operator for readibility, especially if
  return value is ignored
* `FairGeoNode` is a `FairGeoVolume` - no `dynamic_cast` needed
* Explicitely `std::ignore` return values
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    fbc4fe6 View commit details
    Browse the repository at this point in the history

  12. refactor(Base): Deduplicate code into `FairModule::RegisterSensitiveV… 

    …olume()`

* Improve maintainability
    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    e72aa4f View commit details
    Browse the repository at this point in the history

  13. fix(Base): Only increment volume counter if volume is actually added

    @dennisklein
    dennisklein committed May 26, 2024
    Configuration menu
    Copy the full SHA
    135bd40 View commit details
    Browse the repository at this point in the history