Changeset View
Standalone View
src/LauBlattWeisskopfFactor.cc
Show First 20 Lines • Show All 52 Lines • ▼ Show 20 Lines | LauBlattWeisskopfFactor::LauBlattWeisskopfFactor( const LauResonanceInfo& resInfo, const Double_t resRadius, const BarrierType barrierType, const RestFrame restFrame, const BlattWeisskopfCategory category ) : | ||||
radius_(new LauParameter("NEED_A_GOOD_NAME",resRadius,0.0,10.0,kTRUE)), | radius_(new LauParameter("NEED_A_GOOD_NAME",resRadius,0.0,10.0,kTRUE)), | ||||
barrierType_(barrierType), | barrierType_(barrierType), | ||||
restFrame_(restFrame) | restFrame_(restFrame) | ||||
{ | { | ||||
TString categoryName = this->setRadiusName( resInfo, category ); | TString categoryName = this->setRadiusName( resInfo, category ); | ||||
std::cout << "INFO in LauBlattWeisskopfFactor constructor : creating radius parameter for category \"" << categoryName << "\", with initial value " << resRadius << std::endl; | std::cout << "INFO in LauBlattWeisskopfFactor constructor : creating radius parameter for category \"" << categoryName << "\", with initial value " << resRadius << std::endl; | ||||
} | } | ||||
LauBlattWeisskopfFactor::~LauBlattWeisskopfFactor() | LauBlattWeisskopfFactor::LauBlattWeisskopfFactor( const Int_t spin, const Double_t resRadius, const BarrierType barrierType, const RestFrame restFrame, const BlattWeisskopfCategory category ) : | ||||
spin_(spin), | |||||
radius_(new LauParameter("NEED_A_GOOD_NAME",resRadius,0.0,10.0,kTRUE)), | |||||
johndan: Why was `NEED_A_GOOD_NAME` here? | |||||
Done Inline ActionsI think it's just there as an explicit reminder that the name needs to be, and indeed is, updated in the function body. tlatham: I think it's just there as an explicit reminder that the name needs to be, and indeed is… | |||||
barrierType_(barrierType), | |||||
restFrame_(restFrame) | |||||
{ | { | ||||
TString categoryName = this->setRadiusName( category ); | |||||
std::cout << "INFO in LauBlattWeisskopfFactor constructor : creating radius parameter for category \"" << categoryName << "\", with initial value " << resRadius << std::endl; | |||||
} | } | ||||
LauBlattWeisskopfFactor::LauBlattWeisskopfFactor( const LauBlattWeisskopfFactor& other, const UInt_t newSpin, const BarrierType newBarrierType ) : | LauBlattWeisskopfFactor::LauBlattWeisskopfFactor( const LauBlattWeisskopfFactor& other, const UInt_t newSpin, const BarrierType newBarrierType ) : | ||||
spin_(newSpin), | spin_(newSpin), | ||||
radius_(other.radius_->createClone()), | radius_(other.radius_->createClone()), | ||||
barrierType_(newBarrierType), | barrierType_(newBarrierType), | ||||
restFrame_(other.restFrame_) | restFrame_(other.restFrame_) | ||||
{ | { | ||||
} | } | ||||
TString LauBlattWeisskopfFactor::setRadiusName( const LauResonanceInfo& resInfo, const BlattWeisskopfCategory category ) | TString LauBlattWeisskopfFactor::setRadiusName( const LauResonanceInfo& resInfo, const BlattWeisskopfCategory category ) | ||||
{ | { | ||||
TString name = "BarrierRadius_"; | switch (category) { | ||||
TString categoryName; | case Indep : | ||||
return this->setRadiusName( resInfo.getSanitisedName() ); | |||||
default : | |||||
return this->setRadiusName( category ); | |||||
} | |||||
} | |||||
TString LauBlattWeisskopfFactor::setRadiusName( const BlattWeisskopfCategory category ) | |||||
{ | |||||
switch (category) { | switch (category) { | ||||
case Parent : | case Default : | ||||
categoryName = "Parent"; | return this->setRadiusName("Unknown"); | ||||
break; | |||||
case Indep : | case Indep : | ||||
categoryName = resInfo.getSanitisedName(); | // We shouldn't ever end up here | ||||
break; | return this->setRadiusName("Unknown"); | ||||
case Parent : | |||||
return this->setRadiusName("Parent"); | |||||
case Light : | case Light : | ||||
categoryName = "Light"; | return this->setRadiusName("Light"); | ||||
break; | |||||
case Kstar : | case Kstar : | ||||
categoryName = "Kstar"; | return this->setRadiusName("Kstar"); | ||||
break; | |||||
case Charm : | case Charm : | ||||
categoryName = "Charm"; | return this->setRadiusName("Charm"); | ||||
break; | |||||
case StrangeCharm : | case StrangeCharm : | ||||
categoryName = "StrangeCharm"; | return this->setRadiusName("StrangeCharm"); | ||||
break; | |||||
case Charmonium : | case Charmonium : | ||||
categoryName = "Charmonium"; | return this->setRadiusName("Charmonium"); | ||||
break; | |||||
case Beauty : | case Beauty : | ||||
categoryName = "Beauty"; | return this->setRadiusName("Beauty"); | ||||
break; | |||||
case StrangeBeauty : | case StrangeBeauty : | ||||
categoryName = "StrangeBeauty"; | return this->setRadiusName("StrangeBeauty"); | ||||
break; | |||||
case CharmBeauty : | case CharmBeauty : | ||||
categoryName = "CharmBeauty"; | return this->setRadiusName("CharmBeauty"); | ||||
break; | |||||
case Custom1 : | case Custom1 : | ||||
categoryName = "Custom1"; | return this->setRadiusName("Custom1"); | ||||
break; | |||||
case Custom2 : | case Custom2 : | ||||
categoryName = "Custom2"; | return this->setRadiusName("Custom2"); | ||||
break; | |||||
case Custom3 : | case Custom3 : | ||||
categoryName = "Custom3"; | return this->setRadiusName("Custom3"); | ||||
break; | |||||
case Custom4 : | case Custom4 : | ||||
Done Inline ActionsThis seems to be the only case in which resInfo is actually needed, so for all the others I just use a default argument of "" for the category name and then it is assigned in the old way. Passing around "" seems imperfect, and perhaps you'll have a smarter way to do this. What I would rather do is modify the original johndan: This seems to be the only case in which `resInfo` is actually needed, so for all the others I… | |||||
Done Inline ActionsAn alternative way would be to have 3 functions: TString setRadiusName( const LauResonanceInfo& resInfo, const BlattWeisskopfCategory category ); TString setRadiusName( const BlattWeisskopfCategory category ); TString setRadiusName( const TString& categoryName ); The first is modified as you've done except that in the Indep case you just call the 3rd one with the result of resInfo.getSanitisedName(). TString LauBlattWeisskopfFactor::setRadiusName( const TString& categoryName ) { TString name = "BarrierRadius_"; name.Append(categoryName); radius_->name(name); return categoryName; } This avoids having any default arguments but does rather proliferate the number of functions. I agree that these functions should be private, and have no objection to them being moved into that access category. tlatham: An alternative way would be to have 3 functions:
```
TString setRadiusName( const… | |||||
Done Inline ActionsThanks for the detailed proposal! So for the second one, and case Indep:, what does the action look like? You don't have the resInfo to get a name from... Also, what should the name be for the "Default" case? "Unknown", like before, or something else? johndan: Thanks for the detailed proposal!
So for the second one, and `case Indep:`, what does the… | |||||
Done Inline ActionsAlso, why would the second one call the third one, since both have the same form. Why not just set the categoryName variable for the Indep and Default cases? johndan: Also, why would the second one call the third one, since both have the same form. Why not just… | |||||
Done Inline ActionsApologies, I accidentally omitted a few details. To avoid further ambiguities, here's exactly what I had in mind. Hopefully this clarifies what I meant to the extent that all your questions are answered? TString LauBlattWeisskopfFactor::setRadiusName( const LauResonanceInfo& resInfo, const BlattWeisskopfCategory category ) { switch (category) { case Indep : return this->setRadiusName( resInfo.getSanitisedName() ); break; default : return this->setRadiusName( category ); break; } } TString LauBlattWeisskopfFactor::setRadiusName( const BlattWeisskopfCategory category ) { switch (category) { case Default : case Indep : // We shouldn't ever end up here return this->setRadiusName("Unknown"); case Parent : return this->setRadiusName("Parent"); case Light : return this->setRadiusName("Light"); case Kstar : return this->setRadiusName("Kstar"); case Charm : return this->setRadiusName("Charm"); case StrangeCharm : return this->setRadiusName("StrangeCharm"); case Charmonium : return this->setRadiusName("Charmonium"); case Beauty : return this->setRadiusName("Beauty"); case StrangeBeauty : return this->setRadiusName("StrangeBeauty"); case CharmBeauty : return this->setRadiusName("CharmBeauty"); case Custom1 : return this->setRadiusName("Custom1"); case Custom2 : return this->setRadiusName("Custom2"); case Custom3 : return this->setRadiusName("Custom3"); case Custom4 : return this->setRadiusName("Custom4"); } } TString LauBlattWeisskopfFactor::setRadiusName( const TString& categoryName ) { TString name = "BarrierRadius_"; name.Append(categoryName); radius_->name(name); return categoryName; } tlatham: Apologies, I accidentally omitted a few details. To avoid further ambiguities, here's exactly… | |||||
Done Inline ActionsLooks good. The compiler doesn't like the switch with no return outside: /afs/cern.ch/user/j/johndan/ExternalSoftware/Laura++/laura/src/LauBlattWeisskopfFactor.cc: In member function 'TString LauBlattWeisskopfFactor::setRadiusName(LauBlattWeisskopfFactor::BlattWeisskopfCategory)': /afs/cern.ch/user/j/johndan/ExternalSoftware/Laura++/laura/src/LauBlattWeisskopfFactor.cc:124:1: warning: control reaches end of non-void function [-Wreturn-type] 124 | } johndan: Looks good. The compiler doesn't like the switch with no return outside:
```
/afs/cern. | |||||
Done Inline ActionsOh yes, I'd forgotten about gcc's insistence that control can get beyond a switch on an enum that covers every case and returns from each (clang is happy that it can't). // We should never get here but gcc seems to think we can return this->setRadiusName("Unknown"); at the end. tlatham: Oh yes, I'd forgotten about gcc's insistence that control can get beyond a switch on an enum… | |||||
categoryName = "Custom4"; | return this->setRadiusName("Custom4"); | ||||
break; | } | ||||
default : | // We should never get here but gcc seems to think we can | ||||
categoryName = "Unknown"; | return this->setRadiusName("Unknown"); | ||||
break; | |||||
} | } | ||||
TString LauBlattWeisskopfFactor::setRadiusName( const TString& categoryName ) | |||||
{ | |||||
TString name = "BarrierRadius_"; | |||||
name.Append(categoryName); | name.Append(categoryName); | ||||
radius_->name(name); | radius_->name(name); | ||||
return categoryName; | return categoryName; | ||||
} | } | ||||
LauBlattWeisskopfFactor* LauBlattWeisskopfFactor::createClone( const UInt_t newSpin, const BarrierType newBarrierType ) | LauBlattWeisskopfFactor* LauBlattWeisskopfFactor::createClone( const UInt_t newSpin, const BarrierType newBarrierType ) | ||||
{ | { | ||||
LauBlattWeisskopfFactor* clone = new LauBlattWeisskopfFactor( *this, newSpin, newBarrierType ); | LauBlattWeisskopfFactor* clone = new LauBlattWeisskopfFactor( *this, newSpin, newBarrierType ); | ||||
return clone; | return clone; | ||||
} | } | ||||
Show All 21 Lines | if ( barrierType_ == BWBarrier ) { | ||||
} else if (spin_ == 4) { | } else if (spin_ == 4) { | ||||
fFactor = TMath::Sqrt(12746.0*z*z*z*z/(z*z*z*z + 10.0*z*z*z + 135.0*z*z + 1575.0*z + 11025.0)); | fFactor = TMath::Sqrt(12746.0*z*z*z*z/(z*z*z*z + 10.0*z*z*z + 135.0*z*z + 1575.0*z + 11025.0)); | ||||
} else if (spin_ == 5) { | } else if (spin_ == 5) { | ||||
fFactor = TMath::Sqrt(998881.0*z*z*z*z*z/(z*z*z*z*z + 15.0*z*z*z*z + 315.0*z*z*z + 6300.0*z*z + 99225.0*z + 893025.0)); | fFactor = TMath::Sqrt(998881.0*z*z*z*z*z/(z*z*z*z*z + 15.0*z*z*z*z + 315.0*z*z*z + 6300.0*z*z + 99225.0*z + 893025.0)); | ||||
} | } | ||||
} else if ( barrierType_ == BWPrimeBarrier ) { | } else if ( barrierType_ == BWPrimeBarrier ) { | ||||
if (spin_ == 1) { | if (spin_ == 1) { | ||||
fFactor = TMath::Sqrt(1.0/(z + 1.0)); | fFactor = TMath::Sqrt(1.0/(z + 1.0)); | ||||
} else if (spin_ == 2) { | } else if (spin_ == 2) { | ||||
Done Inline ActionsI'm amazed that C++ doesn't have a mechanism to allow enum types to be converted to strings easily. I saw boost has a preprocessor library that can handle it, but managing it in C++ looks messy in the examples I've seen. johndan: I'm amazed that C++ doesn't have a mechanism to allow enum types to be converted to strings… | |||||
Done Inline ActionsIndeed, it is a bit surprising that this isn't easier to do. There are some clever tricks that can be done with macros to autogenerate this stuff but they're hard to read, so I prefer to keep it like this, at least for now. tlatham: Indeed, it is a bit surprising that this isn't easier to do. There are some clever tricks that… | |||||
Done Inline ActionsYes, I entirely agree. The boost solution was the least messy of them all, but certainly not worth introducing dependencies to achieve this minor goal. johndan: Yes, I entirely agree. The boost solution was the least messy of them all, but certainly not… | |||||
fFactor = TMath::Sqrt(1.0/(z*z + 3.0*z + 9.0)); | fFactor = TMath::Sqrt(1.0/(z*z + 3.0*z + 9.0)); | ||||
} else if (spin_ == 3) { | } else if (spin_ == 3) { | ||||
fFactor = TMath::Sqrt(1.0/(z*z*z + 6.0*z*z + 45.0*z + 225.0)); | fFactor = TMath::Sqrt(1.0/(z*z*z + 6.0*z*z + 45.0*z + 225.0)); | ||||
} else if (spin_ == 4) { | } else if (spin_ == 4) { | ||||
fFactor = TMath::Sqrt(1.0/(z*z*z*z + 10.0*z*z*z + 135.0*z*z + 1575.0*z + 11025.0)); | fFactor = TMath::Sqrt(1.0/(z*z*z*z + 10.0*z*z*z + 135.0*z*z + 1575.0*z + 11025.0)); | ||||
} else if (spin_ == 5) { | } else if (spin_ == 5) { | ||||
fFactor = TMath::Sqrt(1.0/(z*z*z*z*z + 15.0*z*z*z*z + 315.0*z*z*z + 6300.0*z*z + 99225.0*z + 893025.0)); | fFactor = TMath::Sqrt(1.0/(z*z*z*z*z + 15.0*z*z*z*z + 315.0*z*z*z + 6300.0*z*z + 99225.0*z + 893025.0)); | ||||
} | } | ||||
Show All 17 Lines |
Why was NEED_A_GOOD_NAME here?