Single shells #539
Single shells #539
Conversation
|
@votca-bot changelog: xtp internally uses only single shells. |
Codecov Report
@@ Coverage Diff @@
## master #539 +/- ##
========================================
- Coverage 54.8% 54.5% -0.3%
========================================
Files 291 291
Lines 30574 30334 -240
========================================
- Hits 16761 16543 -218
+ Misses 13813 13791 -22
Continue to review full report at Codecov.
|
| @@ -187,106 +94,12 @@ BOOST_AUTO_TEST_CASE(large_l_test) { | |||
| // we only check the first and last 600 values because this gets silly quite | |||
| // quickly | |||
| Eigen::Map<Eigen::VectorXd> mapped_result(block.data(), block.size()); | |||
| Eigen::VectorXd ref_head = Eigen::VectorXd::Zero(600); | |||
There was a problem hiding this comment.
removing this vector is great!
| @@ -147,31 +73,12 @@ BOOST_AUTO_TEST_CASE(small_l_test) { | |||
| } | |||
|
|
|||
| BOOST_AUTO_TEST_CASE(large_l_test) { | |||
| std::ofstream xyzfile("C2.xyz"); | |||
|
|
||
| bool CheckShellType(const std::string& shelltype); | ||
| enum class L { S = 0, P = 1, D = 2, F = 3, G = 4, H = 5, I = 6 }; |
There was a problem hiding this comment.
This really improves readability!
| } | ||
| return nbf; | ||
| return ""; |
There was a problem hiding this comment.
If I am not mistaken you will get a compiler warning if you do not treat the whole enum so this cannot happen anymore.
There was a problem hiding this comment.
| } | ||
| return nbf; | ||
| return -1; |
There was a problem hiding this comment.
Is there a case where this -1 could be a problem?
There was a problem hiding this comment.
I think it should never happen.
There was a problem hiding this comment.
I am not sure I did it to get rid of the compiler warning
| case L::H: | ||
| return "H"; | ||
| case L::I: | ||
| return "I"; |
There was a problem hiding this comment.
maybe add a default that throws an error, same for the switches that follow this one
There was a problem hiding this comment.
yes I agree. I hope that it actually produces a compiler warning if you do not treat all the enum cases, so that you do not need a default.
There was a problem hiding this comment.
https://stackoverflow.com/questions/37254496/c-warning-enumeration-value-not-handled-in-switch-wswitch actually without the default you get a compiler warning, which I prefer.
There was a problem hiding this comment.
Me to, didn't know about that
implements #538
Shells do now have a single enum which tells their L value.