What is the argument in WCSimTrajectory::DrawTrajectory() supposed to do?

Warnings in WCSimTrajectory

Warning text

Compiling WCSimTrajectory.cc ...
In file included from include/WCSimTrajectory.hh:6:0,
                 from src/WCSimTrajectory.cc:1:
/data/neutrino06/pspradlin/hk-hyperk/Geant4/install/include/Geant4/G4VTrajectory.hh:91:17: warning: ‘virtual void G4VTrajectory::DrawTrajectory() const’ was hidden [-Woverloaded-virtual]
    virtual void DrawTrajectory() const;
                 ^
In file included from src/WCSimTrajectory.cc:1:0:
include/WCSimTrajectory.hh:78:17: warning:   by ‘virtual void WCSimTrajectory::DrawTrajectory(G4int) const’ [-Woverloaded-virtual]
    virtual void DrawTrajectory(G4int i_mode=0) const;
                 ^
src/WCSimTrajectory.cc:96:6: warning: unused parameter ‘i_mode’ [-Wunused-parameter]
 void WCSimTrajectory::DrawTrajectory(G4int i_mode) const
      ^
src/WCSimTrajectory.cc: In member function ‘virtual std::vector<G4AttValue>* WCSimTrajectory::CreateAttValues() const’:
src/WCSimTrajectory.cc:140:25: warning: declaration of ‘s’ shadows a global declaration [-Wshadow]
   std::ostringstream s(c);
                         ^
In file included from /data/neutrino06/pspradlin/hk-hyperk/Geant4/install/include/Geant4/CLHEP/Units/PhysicalConstants.h:42:0,
                 from /data/neutrino06/pspradlin/hk-hyperk/Geant4/install/include/Geant4/G4ParticleDefinition.hh:58,
                 from include/WCSimTrajectory.hh:12,
                 from src/WCSimTrajectory.cc:1:
/data/neutrino06/pspradlin/hk-hyperk/Geant4/install/include/Geant4/CLHEP/Units/SystemOfUnits.h:141:24: warning: shadowed declaration is here [-Wshadow]
   static const double  s = second;
                        ^

There are some interesting things going on here.

Shadowing `s`

The CLHEP SystemOfUnits.h puts a lot variables in the global namespace that represent common units. Remember to avoid variable names that defined names for units. Here, the local variable that shadows the global unit s is a stringstream text buffer. Rename it to st. I like the compactness of the one-letter variable name, but i do not see a nice one-letter substitute that makes the distinction between string and char as well as the current implementation's s and c do.

WCSimTrajectory::DrawTrajectory()

This is interesting. The WCSimTrajectory::DrawTrajectory() method overloads a method that is defined in the base class G4VTrajectory. The derived instance looks like it is a placeholder for something that hasn't been implemented. At the moment, it just invokes the base class method:

void WCSimTrajectory::DrawTrajectory(G4int i_mode) const
{
  // Invoke the default implementation in G4VTrajectory...
  G4VTrajectory::DrawTrajectory();
  // ... or override with your own code here.
}

Although the argument for the derived class method does nothing, the only invocation DrawTrajectory() passes an argument to it (in WCSimEventAction::EndOfEventAction()). Further, although the name of the argument, i_mode, suggests some kind of modal behavior, the value 50 passed into it is not obviously a modal index used anywhere else in WCSimEventAction::EndOfEventAction().

I cannot make an easy fix to this. This probably deserves its own issue to discuss what it is doing and what it is supposed to be doing.

Inconclusive

I have created an issue stub to follow-up on this.

Originally posted by @spradlin in https://github.com/spradlin/WCSim/issues/9#issuecomment-1001859817

spradlin / WCSim