Apply good practices in the project

README.md

@@ -42,22 +42,22 @@ Information about all fields in all branches is stored in Configuration object.
 ROOT6 is needed for installation.
 Follow CERN ROOT [instructions](https://root.cern/install/) to install it.
 Version compiled with c++17 flag is preferred, otherwise CMAKE_CXX_STANDARD flag needs to be explicitly specified (see below).
-
+```bash
     git clone https://github.com/HeavyIonAnalysis/AnalysisTree.git
     cd AnalysisTree
     mkdir build install
     cd build
     source path-to-root-installation/bin/thisroot.sh
     cmake -DCMAKE_INSTALL_PREFIX=../install ../
     make -j install
-
+```
 *path-to-root-installation* must be replaced with your actual location of Root install directory.
 
 ### List of CMake options:
 To apply the flag use -D{Name}={value}, for example, if you want to compile using c++11:
-
+```bash    
     cmake -DCMAKE_CXX_STANDARD=11 ../
-
+```
 | Name  | Default value | Possible values |
 | ------------- | ------------- | ---------- |
 | CMAKE_BUILD_TYPE  | RELEASE  | RELEASE/DEBUG |
@@ -70,16 +70,16 @@ To apply the flag use -D{Name}={value}, for example, if you want to compile usin
 ### Setting AnalysisTree environment
 Whatever you are going to do with AnalysisTree - read the file, perform analysis based on information stored in it or create your own file, first of all you need to set up environment variables.
 It can be done in a single command:
-
+```bash
     source path-to-analysis_tree-installation/bin/AnalysisTreeConfig.sh
-
+```
 ### Reading files from ROOT session
 An example of AnalysisTree ROOT file can be downloaded by this [link](https://sf.gsi.de/f/3ba5a9e3ff5248edba2c/?dl=1)
 
 Open a ROOT-file
-
+```bash
     root -l 1.analysistree.root
-
+```
 Check its content
 
     .ls
@@ -98,9 +98,9 @@ and, finally, AnalysisTree::DataHeader object named *DataHeader* containing info
 
 #### Reading the configuration
 In order to know the structure of the tree, perform following command:
-
+```c++
     Configuration->Print()
-
+```
 An output will contain plenty of branches and matchings between them.
 Let us look at one of them:
 
@@ -114,7 +114,7 @@ Negative ids belong to default fields of branches while positive ids and 0 - to
 Middle column contains string name of the field, and right column - a description of it.
 
 #### Digesting the tree content
-
+```c++
     Configuration->GetBranchConfig("SimParticles").GetType()
     // to know to which type (Hit, Track, Module, Particle or EventHeader belongs SimParticles branch)
 
@@ -149,27 +149,27 @@ Middle column contains string name of the field, and right column - a descriptio
     // drawing a histogram with user-defined binning and ranges.
 
     rTree->Draw("TMath::Log(TMath::Abs(SimParticles.channels_.GetPx()))")
-    // drawing a distribution of derived quantites calculated by formula
-
+    // drawing a distribution of derived quantities calculated by formula
+```
 Moreover, for default fields which are explicitly present in Container (i.e. px is OK, but not pt, which is not stored but calculated on fly) there is a possibility to draw them using TTree::Draw syntax:
-
+```c++
     rTree->Draw("SimParticles.px_")
     rTree->Draw("TMath::Log(TMath::Abs(SimParticles.px_))")
     rTree->Draw("TMath::Log(TMath::Abs(SimParticles.px_)) * TMath::Cos(SimParticles.py_)")
-
+```
 Also you can open a ROOT interactive session and create an AnalysisTree::Chain:
-
+```c++
     AnalysisTree::Chain t("1.analysistree.root", "rTree") // Chain constructor with a single file
     // or
     AnalysisTree::Chain t({"filelist.txt"}, {"rTree"})  // Chain constructor with a file list
-
+```
 and then build any fields including user-defined and implicitly present fields (such as phi or pt):
-
+```c++
     t.Draw("SimParticles.px")
     t.Draw("SimParticles.pid")
     t.Draw("VtxTracks.chi2")
     // 2D histograms, cuts, drawing options and math formulas mentioned above are also available
-
+```
 ### Reading file with a macro
 
 Building of distributions in interactive mode is a good approach only if the commands are simple, and the number of events to be analized is not so big.

core/BranchConfig.cpp

@@ -14,7 +14,7 @@ void BranchConfig::GenerateId() {
   id_ = id_hasher(name_);
 }
 
-BranchConfig::BranchConfig(std::string name, DetType type, std::string title) : name_(std::move(name)), type_(type), title_(std::move(title)) {
+BranchConfig::BranchConfig(std::string name, DetType type, std::string title) : name_(std::move(name)), title_(std::move(title)), type_(type) {
   GenerateId();
 
   if (type_ == DetType::kTrack) {
@@ -156,12 +156,12 @@ void VectorConfig<T>::Print() const {
     std::vector<std::string> result;
     std::vector<int> newlinepositions{-1};
     int it{0};
-    while (it < std::string::npos) {
+    while (it < static_cast<int>(std::string::npos)) {
       it = input.find("\n", it + 1);
       newlinepositions.emplace_back(it);
     }
     newlinepositions.back() = input.size();
-    for (int ip = 0; ip < newlinepositions.size() - 1; ++ip) {
+    for (int ip = 0; ip < static_cast<int>(newlinepositions.size()) - 1; ++ip) {
       result.emplace_back(input.substr(newlinepositions.at(ip) + 1, newlinepositions.at(ip + 1) - newlinepositions.at(ip) - 1));
     }
     return result;
@@ -174,7 +174,7 @@ void VectorConfig<T>::Print() const {
     } else {
       auto est = SplitString(entry.second.title_);
       print_row({{std::to_string(entry.second.id_), 10}, {entry.first, name_strlen}, {est.at(0), 50}});
-      for (int iest = 1; iest < est.size(); ++iest) {
+      for (int iest = 1; iest < static_cast<int>(est.size()); ++iest) {
         print_row({{"", 10}, {"", name_strlen}, {est.at(iest), 50}});
       }
     }

core/Configuration.cpp

@@ -19,14 +19,12 @@ namespace AnalysisTree {
 
 void Configuration::Streamer(TBuffer& rb) {
   if (rb.IsReading()) {
-    bool read_ok = false;
     UInt_t rs = 0, rc = 0;
     auto version_from_file = rb.ReadVersion(&rs, &rc, Configuration::Class());
     if (version_from_file == Class()->GetClassVersion()) {
       Configuration::Class()->ReadBuffer(rb, this, version_from_file, rs, rc);
       // populate the transient field
       this->matches_index_ = MakeMatchingIndex(matches_);
-      read_ok = true;
     } else if (version_from_file == 3) {
       //      below structure description for version 3 of this class
       Warning(__func__, "Reading AnalysisTree::Configuration of version %d in compatibility mode. "
@@ -40,14 +38,12 @@ void Configuration::Streamer(TBuffer& rb) {
       this->matches_ = MakeMatchConfigsFromIndex(conf_v3.matches_);
       // populate the transient field
       this->matches_index_ = conf_v3.matches_;
-      read_ok = true;
     } else {
       Warning(__func__, "Current version of AnalysisTree::Configuration (%d) "
                         "is different from the version from file (%d) and no rule to read this version was implemented. "
                         "Contact developers if dedicated Streamer implementation is needed for this version.",
               Configuration::Class()->GetClassVersion(),
               version_from_file);
-      read_ok = false;
     }
 
   } else {

core/Configuration.test.cpp

@@ -30,9 +30,6 @@ TEST(Configuration, Basics) {
 
   EXPECT_EQ(config.GetNumberOfBranches(), 1);
   //  EXPECT_EQ(config.GetLastId(), 0);
-
-  const auto& br1 = config.GetBranchConfig("RecTrack");
-  //  EXPECT_EQ(br1.GetId(), 0);
 }
 
 TEST(Configuration, Match) {

core/Particle.hpp

@@ -102,7 +102,7 @@ class Particle : public Track {
   PdgCode_t pid_{0};
   bool is_allowed_set_charge_and_mass_explicitly_{false};//!
 
-  ClassDefOverride(Particle, 2);
+  ClassDefOverride(Particle, 3);
 };
 
 }// namespace AnalysisTree

core/Track.hpp

@@ -164,7 +164,7 @@ class Track : public Container {
   Floating_t pz_{UndefValueFloat};///< z-component of track's momentum
   Integer_t charge_{-1000};
 
-  ClassDefOverride(Track, 2);
+  ClassDefOverride(Track, 3);
 };
 
 }// namespace AnalysisTree

examples/example.cpp

@@ -1,8 +1,8 @@
 /* Copyright (C) 2019-2021 GSI, Universität Tübingen
    SPDX-License-Identifier: GPL-3.0-only
    Authors: Viktor Klochkov, Ilya Selyuzhenkov */
-#include <iostream>
 #include <Chain.hpp>
+#include <iostream>
 
 using namespace AnalysisTree;
 
@@ -23,7 +23,7 @@ int main(int argc, char* argv[]) {
   return 0;
 }
 
-void example(const std::string& filename, const std::string& treename){
+void example(const std::string& filename, const std::string& treename) {
   auto* chain = new Chain(filename, treename);
   chain->InitPointersToBranches({"VtxTracks", "SimParticles"});
 
@@ -34,13 +34,12 @@ void example(const std::string& filename, const std::string& treename){
   config->Print();
 
   auto rec_particles = chain->GetBranchObject("VtxTracks");
-  auto rec2sim_particles = chain->GetMatching("VtxTracks", "SimParticles");
 
   auto rec_pT = rec_particles.GetField("pT");
 
   for (long i_event = 0; i_event < 10; ++i_event) {
     chain->GetEntry(i_event);
-    for(size_t i=0; i<rec_particles.size(); ++i){
+    for (size_t i = 0; i < rec_particles.size(); ++i) {
       auto pT = rec_particles[i][rec_pT];
       std::cout << " track #" << i << " pT = " << pT << std::endl;
     }

infra/AnalysisEntry.cpp

@@ -32,7 +32,7 @@ bool AnalysisEntry::ApplyCutOnBranches(std::vector<const Branch*>& br, std::vect
   std::vector<size_t> id_vec;
   bch_vec.reserve(br.size());
   id_vec.reserve(br.size());
-  for (int i = 0; i < br.size(); i++) {
+  for (int i = 0; i < static_cast<int>(br.size()); i++) {
     BranchChannel* bchptr = new BranchChannel(br.at(i), ch.at(i));
     if (cuts.at(i) != nullptr) {
       if (!cuts.at(i)->Apply(*bchptr)) {
@@ -71,7 +71,7 @@ double AnalysisEntry::FillVariable(const Variable& var, std::vector<const Branch
   std::vector<size_t> id_vec;
   bch_vec.reserve(br.size());
   id_vec.reserve(br.size());
-  for (int i = 0; i < br.size(); i++) {
+  for (int i = 0; i < static_cast<int>(br.size()); i++) {
     BranchChannel* bchptr = new BranchChannel(br.at(i), id.at(i));
     bch_vec.emplace_back(bchptr);
     id_vec.emplace_back(br.at(i)->GetId());
@@ -83,6 +83,10 @@ double AnalysisEntry::FillVariable(const Variable& var, std::vector<const Branch
   return result;
 }
 
+double AnalysisEntry::FillWeight(const Variable& var, std::vector<const Branch*>& br, std::vector<int>& id) {
+  return var.GetNumberOfBranches() > 0 ? FillVariable(var, br, id) : 1.;
+}
+
 double AnalysisEntry::FillVariable(const Variable& var, const Branch& br1, int ch1, const Branch& br2, int ch2) {
   Branch* br1_ptr = new Branch(std::move(br1));
   Branch* br2_ptr = new Branch(std::move(br2));
@@ -132,7 +136,7 @@ void AnalysisEntry::FillFromEveHeaders() {
     i_var++;
   }//variables
   values_.emplace_back(temp_vars);
-  weights_.emplace_back(FillVariable(var4weight_, br_vec, id_vec));
+  weights_.emplace_back(FillWeight(var4weight_, br_vec, id_vec));
 }
 
 /**
@@ -168,7 +172,7 @@ void AnalysisEntry::FillFromOneChannalizedBranch() {
       i_var++;
     }//variables
     values_.emplace_back(temp_vars);
-    weights_.emplace_back(FillVariable(var4weight_, br_vec, id_vec));
+    weights_.emplace_back(FillWeight(var4weight_, br_vec, id_vec));
   }// channels
 }
 
@@ -211,7 +215,7 @@ void AnalysisEntry::FillFromTwoChannalizedBranches() {
       i_var++;
     }//variables
     values_.emplace_back(temp_vars);
-    weights_.emplace_back(FillVariable(var4weight_, br_vec, id_vec));
+    weights_.emplace_back(FillWeight(var4weight_, br_vec, id_vec));
   }// channels
 }
 

infra/AnalysisEntry.hpp

@@ -29,7 +29,6 @@ class AnalysisEntry {
   explicit AnalysisEntry(std::vector<Variable> vars, Cuts* cuts = nullptr, Variable vars4weight = {}) : vars_(std::move(vars)),
                                                                                                         var4weight_(std::move(vars4weight)),
                                                                                                         cuts_(cuts) {
-    var4weight_.IfEmptyVariableConvertToOnes(vars_.at(0));
     FillBranchNames();
   };
 
@@ -62,6 +61,7 @@ class AnalysisEntry {
   ANALYSISTREE_ATTR_NODISCARD bool ApplyCutOnBranches(std::vector<const Branch*>& br, std::vector<Cuts*>& cuts, std::vector<int>& ch) const;
   [[deprecated]] ANALYSISTREE_ATTR_NODISCARD bool ApplyCutOnBranches(const Branch& br1, Cuts* cuts1, int ch1, const Branch& br2, Cuts* cuts2, int ch2) const;
   static double FillVariable(const Variable& var, std::vector<const Branch*>& br, std::vector<int>& id);
+  static double FillWeight(const Variable& var, std::vector<const Branch*>& br, std::vector<int>& id);
   [[deprecated]] static double FillVariable(const Variable& var, const Branch& br1, int ch1, const Branch& br2, int ch2);
 
   std::vector<Variable> vars_{};

infra/Branch.cpp

@@ -169,6 +169,12 @@ void Branch::CopyContentsRaw(Branch* other) {
       *ANALYSISTREE_UTILS_GET<EventHeader*>(data_) = *ANALYSISTREE_UTILS_GET<EventHeader*>(other->data_);
       break;
     }
+    case DetType::kGeneric: {
+      *ANALYSISTREE_UTILS_GET<GenericDetector*>(data_) = *ANALYSISTREE_UTILS_GET<GenericDetector*>(other->data_);
+      break;
+    }
+    default:
+      throw std::runtime_error("Branch::CopyContentsRaw(): unexpected branch type of 'this'");
   }
 }
 

infra/BranchChannel.cpp

@@ -17,7 +17,7 @@ void BranchChannel::UpdateChannel(size_t new_channel) {
 }
 
 void BranchChannel::UpdatePointer() {
-  if (i_channel_ >= 0 && i_channel_ < branch_->size()) {
+  if (i_channel_ < branch_->size()) {
     data_ptr_ = ANALYSISTREE_UTILS_VISIT(get_channel_struct(i_channel_), branch_->GetData());
   } else {
     throw std::out_of_range("");

infra/GenericContainerFiller.cpp

@@ -55,7 +55,7 @@ void GenericContainerFiller::Init() {
 
   config_.AddBranchConfig(branchConfig);
 
-  for (int iV = 0; iV < branch_values_.size(); iV++) {
+  for (int iV = 0; iV < static_cast<int>(branch_values_.size()); iV++) {
     SetAddressFICS(branch_map_.at(iV).name_, branch_map_.at(iV), branch_values_.at(iV));
   }
 
@@ -95,8 +95,8 @@ void GenericContainerFiller::Finish() {
 void GenericContainerFiller::Run(int nEntries) {
   Init();
 
-  const size_t nTreeEntries = tree_in_->GetEntries();
-  const size_t nRunEntries = (nEntries < 0 || nEntries > nTreeEntries) ? nTreeEntries : nEntries;
+  const int nTreeEntries = tree_in_->GetEntries();
+  const int nRunEntries = (nEntries < 0 || nEntries > nTreeEntries) ? nTreeEntries : nEntries;
 
   int previousTriggerVar{-799};
   for (int iEntry = 0; iEntry < nRunEntries; iEntry++) {
@@ -108,8 +108,8 @@ void GenericContainerFiller::Run(int nEntries) {
 }
 
 int GenericContainerFiller::DetermineFieldIdByName(const std::vector<IndexMap>& iMap, const std::string& name) {
-  auto distance = std::distance(iMap.begin(), std::find_if(iMap.begin(), iMap.end(), [&name](const IndexMap& p) { return p.name_ == name; }));
-  if (distance == iMap.size()) throw std::runtime_error("DetermineFieldIdByName(): name " + name + " is missing");
+  const auto distance = static_cast<int>(std::distance(iMap.begin(), std::find_if(iMap.begin(), iMap.end(), [&name](const IndexMap& p) { return p.name_ == name; })));
+  if (distance == static_cast<int>(iMap.size())) throw std::runtime_error("DetermineFieldIdByName(): name " + name + " is missing");
   return distance;
 }
 
@@ -126,7 +126,7 @@ void GenericContainerFiller::SetAddressFICS(const std::string& branchName, const
 }
 
 void GenericContainerFiller::SetFieldsFICS(const std::vector<IndexMap>& imap, Container& container, const std::vector<FICS>& ficc) {
-  for (int iV = 0; iV < ficc.size(); iV++) {
+  for (int iV = 0; iV < static_cast<int>(ficc.size()); iV++) {
     if (imap.at(iV).field_type_ == "TLeafF") container.SetField(ficc.at(iV).float_, imap.at(iV).index_);
     else if (imap.at(iV).field_type_ == "TLeafI")
       container.SetField(ficc.at(iV).int_, imap.at(iV).index_);
@@ -137,4 +137,4 @@ void GenericContainerFiller::SetFieldsFICS(const std::vector<IndexMap>& imap, Co
     else
       throw std::runtime_error("GenericContainerFiller::SetFieldsFICS(): unsupported filed type " + imap.at(iV).field_type_);
   }
-}
+}