framework icon indicating copy to clipboard operation
framework copied to clipboard

Migrate drawing and analysis methods to dedicated classes or namespaces

Open juanangp opened this issue 3 years ago • 4 comments

Migrate drawing and analysis methods within the different libraries to a namespace such as TRestXXXPainter and TRestXXXAnalysis:

  • TRestXXXPainter should implement all the drawing methods within the XXX library.
  • TRestXXXAnalysis should implement all the analysis methods within the XXX library.

All the libraries inside the repository should be migrated to this schema. Perhaps, the implementation of TRestHitsPainter and TRestHitsAnalysis inside framework can be discussed. We should assing a developer to migrate the different libraries:

Ideally a template should be provided to use a similar schema for all the libraries by defining dedicated TRestXXXPainter and TRestXXXAnalysis namespaces in every single library.

We should discuss the need of other generic namespace such as TRestXXXTools

juanangp avatar Apr 19 '22 11:04 juanangp

This is a great idea! I can do geant4lib and possible more, but first we should provide an example as you say.

I am not sure if I understand the proposal exactly, do you mean to move all methods to static and place them in another namespace? or to define corresponding class methods in a different namespace?

lobis avatar Apr 19 '22 11:04 lobis

Perhaps we could then re-organize TRestTools and TRestStringHelper and move to something like TRestFiletools TRestStringTools, TRestFrameworkToolsm TRestTools, ...

jgalan avatar Apr 19 '22 11:04 jgalan

This is a great idea! I can do geant4lib and possible more, but first we should provide an example as you say.

I am not sure if I understand the proposal exactly, do you mean to move all methods to static and place them in another namespace? or to define corresponding class methods in a different namespace?

I meant to move al methods to a dedicated class namespace TRestXXXPainter and TRestXXXAnalysis. I think that while doing that, we don't have to define the functions as static. Something like that:

//TRestTrackPainter.h
# include <TRestTrackEvent.h>
#ifndef TRestTrack_Painter
#define TRestTrack_Painter

namespace TRestTrackPainter {
  void GetOriginEnd(TRestTrackEvent* trackEvent, std::vector<TGraph*>& originGr,
                             std::vector<TGraph*>& endGr, std::vector<TLegend*>& leg);
}
#endif
//TRestTrackPainter.cxx
# include <TRestTrackPainter.h>

  void TRestTrackPainter::GetOriginEnd(TRestTrackEvent* trackEvent, std::vector<TGraph*>& originGr,
                             std::vector<TGraph*>& endGr, std::vector<TLegend*>& leg){
     //Implement the method here
  }

Then you can do TRestTrackPainter::GetOriginEnd(track, origGr, endGr, leg) from elsewhere.

juanangp avatar Apr 19 '22 11:04 juanangp

Perhaps we could then re-organize TRestTools and TRestStringHelper and move to something like TRestFiletools TRestStringTools, TRestFrameworkToolsm TRestTools, ...

Indeed, this should be definetively done, but perhaps in a different issue? I am afraid this issue is already too large.

Also, I would suggest to re-visit TRestStringOutput and perhaps rename as TRestStreamer. I think we should rename the warning to RESTWarning (same applies to debug,info,error,...) and we should implement RESTendl instead of using a custom endl which can be mis-identified with std::endl.

juanangp avatar Apr 19 '22 12:04 juanangp