Add a factory member to BiorthBasis#157
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds factory methods to the BiorthBasis class to eliminate the need for dynamic casting when creating instances. The change provides a cleaner API by allowing direct instantiation of BiorthBasis objects without going through the base Basis class factory.
- Implements
factoryandfactory_stringstatic methods inBiorthBasisclass - Updates error messages in
BasisFactory.ccto be more consistent - Includes improvements to the Sturm-Liouville grid solver with boundary heuristics
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| expui/BiorthBasis.H | Adds factory method declarations to BiorthBasis class |
| expui/BiorthBasis.cc | Implements factory methods with type-specific instantiation logic |
| expui/BasisFactory.cc | Updates error messages for consistency |
| include/SLGridMP2.H | Adds static member variables for radial boundary heuristics |
| exputil/SLGridMP2.cc | Implements improved Sturm-Liouville solver with adaptive grid boundaries |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Does this change anything for the |
I could have the |
|
After discussions with @lgarrison:
|
The issue
The base class,
Basisprovides a factory member that returns astd::shared_ptr<Basis>. Using any of the interface specific toBiorthBasisrequires a dynamic cast. For example, to use thegetAccel()member, one needs:as discovered in discussion with @lgarrison
A cleaner solution
Enable factory methods for
BiorthBasisdirectly. Then, the code block above becomes:The end result will be identical but the intent is precise. This entire PR is syntactic sugar but we might as well present a nicer API.
Notes
BiorthBasisheader. I suspect that I intended to implement these as part of the original design ofBiorthBasisbut it fell off the ToDo stack.