Skip to content

Commit 9e47c03

Browse files
Critsium-xyclaude
andauthored
source_basis/module_ao: last access hack in the module, via accessors plus one friend (#7965)
`orb_nonlocal_lm_test.cpp` is the last file in `module_ao` carrying `#define private public`. It reaches 78 times into `Numerical_Nonlocal_Lm` from its TEST_F bodies. Most of that needs nothing from the class: ten of the members already have public accessors, so `nnl[ip].nr` becomes `nnl[ip].getNr()`, `.r_radial[ir]` becomes `.getRadial(ir)`, `.beta_k[ik]` becomes `.getBeta_k(ik)`, and so on. Six things have no public route and get `friend class NumericalNonlocalLmTest;` plus thin read-only forwarders on the fixture: the members `label`, `kcut`, `index_proj` and `rab`, and the private methods `freemem()` and `renew()`, which the FreeAndRenew test exists to exercise. Friendship is not inherited and a TEST_F body lives in a derived class, hence the forwarders -- the arrangement established in #7949. Nothing that mutates state is exposed. The r->k->r round-trip check does reach deep -- it swaps the r-space and k-space arrays of a projector, reallocates `rab`, and calls the private `get_kradial()` -- but that code lives in `NumericalNonlocalLmTest::err_r2k2r`, a member of the fixture itself, so the friend declaration covers it and it is unchanged. 28 of the 106 total accesses are inside fixture helpers like that one and needed no edit. No test expectation changed; every substitution reads the same member through the accessor that returns it. With this, `module_ao` has no `#define private public` left. Occurrences tree-wide 84 -> 83. Production change is four lines: a forward declaration and one friend. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent dcb849e commit 9e47c03

2 files changed

Lines changed: 80 additions & 66 deletions

File tree

‎source/source_basis/module_ao/orb_nonlocal_lm.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,13 @@
1515
* DATE : 2008-03-04
1616
*/
1717

18+
class NumericalNonlocalLmTest;
19+
1820
class Numerical_Nonlocal_Lm
1921
{
22+
/// the unit test drives freemem(), renew() and get_kradial() directly, and
23+
/// reads label/kcut/index_proj/rab, which have no public accessor
24+
friend class NumericalNonlocalLmTest;
2025

2126
public:
2227

‎source/source_basis/module_ao/test/orb_nonlocal_lm_test.cpp‎

Lines changed: 75 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@
55
#include <fstream>
66
#include <iomanip>
77

8-
#define private public
98
#include "source_basis/module_ao/orb_nonlocal_lm.h"
10-
#undef private
119

1210

1311
#ifdef __MPI
@@ -66,6 +64,17 @@ class NumericalNonlocalLmTest : public ::testing::Test
6664
void change_k(Numerical_Nonlocal_Lm&, double const& ecut, double const& dk);
6765
bool check_file_match(size_t const& nline, double const* col1, double const* col2, double const& tol, std::string const& fname);
6866

67+
// Numerical_Nonlocal_Lm declares this fixture a friend, but a TEST_F body
68+
// lives in a class derived from it and friendship is not inherited. These
69+
// reach the members that have no public accessor, and the private methods.
70+
// Everything with an accessor is read through it instead.
71+
static const std::string& label_of(const Numerical_Nonlocal_Lm& o) { return o.label; }
72+
static int index_proj_of(const Numerical_Nonlocal_Lm& o) { return o.index_proj; }
73+
static double kcut_of(const Numerical_Nonlocal_Lm& o) { return o.kcut; }
74+
static const double* rab_of(const Numerical_Nonlocal_Lm& o) { return o.rab; }
75+
static void freemem(Numerical_Nonlocal_Lm& o) { o.freemem(); }
76+
static void renew(Numerical_Nonlocal_Lm& o) { o.renew(); }
77+
6978
// number of beta projectors
7079
size_t nproj_;
7180

@@ -380,27 +389,27 @@ TEST_F(NumericalNonlocalLmTest, Init) {
380389
this->init();
381390

382391
for (size_t ip = 0; ip != nproj_; ++ip) {
383-
EXPECT_EQ(elem_label_, nnl[ip].label);
384-
EXPECT_EQ(index_atom_type_, nnl[ip].index_atom_type);
385-
EXPECT_EQ(l_[ip], nnl[ip].angular_momentum_l);
392+
EXPECT_EQ(elem_label_, label_of(nnl[ip]));
393+
EXPECT_EQ(index_atom_type_, nnl[ip].getType());
394+
EXPECT_EQ(l_[ip], nnl[ip].getL());
386395
EXPECT_EQ(dr_uniform_, nnl[ip].dr_uniform);
387-
EXPECT_EQ(nr_[ip], nnl[ip].nr);
388-
EXPECT_EQ(r_radial_[nr_[ip]-1], nnl[ip].rcut);
389-
EXPECT_EQ(nk_, nnl[ip].nk);
390-
EXPECT_EQ(dk_, nnl[ip].dk);
391-
396+
EXPECT_EQ(nr_[ip], nnl[ip].getNr());
397+
EXPECT_EQ(r_radial_[nr_[ip]-1], nnl[ip].getRcut());
398+
EXPECT_EQ(nk_, nnl[ip].getNk());
399+
EXPECT_EQ(dk_, nnl[ip].getDk());
400+
392401
// freemem() & renew() will be tested elsewhere
393402

394403
for (int ir = 0; ir != nr_[ip]; ++ir) {
395-
EXPECT_EQ(r_radial_[ir], nnl[ip].r_radial[ir]);
396-
EXPECT_EQ(rab_[ir], nnl[ip].rab[ir]);
397-
EXPECT_EQ(beta_r_[ip][ir], nnl[ip].beta_r[ir]);
404+
EXPECT_EQ(r_radial_[ir], nnl[ip].getRadial(ir));
405+
EXPECT_EQ(rab_[ir], rab_of(nnl[ip])[ir]);
406+
EXPECT_EQ(beta_r_[ip][ir], nnl[ip].getBeta_r(ir));
398407
}
399408

400409
for (size_t ik = 0; ik != nk_; ++ik) {
401-
EXPECT_EQ(ik*dk_, nnl[ip].k_radial[ik]);
410+
EXPECT_EQ(ik*dk_, nnl[ip].getKpoint(ik));
402411
}
403-
EXPECT_EQ((nk_-1)*dk_, nnl[ip].kcut);
412+
EXPECT_EQ((nk_-1)*dk_, kcut_of(nnl[ip]));
404413

405414
// get_kradial() will be tested elsewhere
406415
}
@@ -422,13 +431,13 @@ TEST_F(NumericalNonlocalLmTest, Getters) {
422431
ASSERT_NE(nnl[iproj].getBeta_r(), nullptr);
423432
ASSERT_NE(nnl[iproj].getBeta_k(), nullptr);
424433

425-
for (int ir = 0; ir != nnl[iproj].nr; ++ir) {
434+
for (int ir = 0; ir != nnl[iproj].getNr(); ++ir) {
426435
EXPECT_DOUBLE_EQ(nnl[iproj].getRadial(ir), 0.01*ir);
427436
EXPECT_DOUBLE_EQ(nnl[iproj].getRadial()[ir], 0.01*ir);
428437
EXPECT_DOUBLE_EQ(nnl[iproj].getBeta_r()[ir], nnl[iproj].getBeta_r(ir));
429438
}
430439

431-
for (int ik = 0; ik != nnl[iproj].nk; ++ik) {
440+
for (int ik = 0; ik != nnl[iproj].getNk(); ++ik) {
432441
EXPECT_DOUBLE_EQ(nnl[iproj].getKpoint(ik), ik*0.01);
433442
EXPECT_DOUBLE_EQ(nnl[iproj].getKpoint()[ik], ik*0.01);
434443
EXPECT_DOUBLE_EQ(nnl[iproj].getBeta_k()[ik], nnl[iproj].getBeta_k(ik));
@@ -479,31 +488,31 @@ TEST_F(NumericalNonlocalLmTest, DeepCopy) {
479488
size_t iproj = 3;
480489
tmp = nnl[iproj];
481490

482-
EXPECT_EQ(tmp.label, nnl[iproj].label);
483-
EXPECT_EQ(tmp.index_atom_type, nnl[iproj].index_atom_type);
484-
EXPECT_EQ(tmp.angular_momentum_l, nnl[iproj].angular_momentum_l);
485-
EXPECT_EQ(tmp.nr, nnl[iproj].nr);
486-
EXPECT_EQ(tmp.nk, nnl[iproj].nk);
487-
EXPECT_EQ(tmp.index_proj, nnl[iproj].index_proj);
491+
EXPECT_EQ(label_of(tmp), label_of(nnl[iproj]));
492+
EXPECT_EQ(tmp.getType(), nnl[iproj].getType());
493+
EXPECT_EQ(tmp.getL(), nnl[iproj].getL());
494+
EXPECT_EQ(tmp.getNr(), nnl[iproj].getNr());
495+
EXPECT_EQ(tmp.getNk(), nnl[iproj].getNk());
496+
EXPECT_EQ(index_proj_of(tmp), index_proj_of(nnl[iproj]));
488497

489-
EXPECT_DOUBLE_EQ(tmp.rcut, nnl[iproj].rcut);
490-
EXPECT_DOUBLE_EQ(tmp.kcut, nnl[iproj].kcut);
491-
EXPECT_DOUBLE_EQ(tmp.dk, nnl[iproj].dk);
498+
EXPECT_DOUBLE_EQ(tmp.getRcut(), nnl[iproj].getRcut());
499+
EXPECT_DOUBLE_EQ(kcut_of(tmp), kcut_of(nnl[iproj]));
500+
EXPECT_DOUBLE_EQ(tmp.getDk(), nnl[iproj].getDk());
492501

493502
ASSERT_NE(tmp.getRadial(), nullptr);
494503
ASSERT_NE(tmp.getKpoint(), nullptr);
495504
ASSERT_NE(tmp.getBeta_k(), nullptr);
496505
ASSERT_NE(tmp.getBeta_r(), nullptr);
497506

498-
for (int ir = 0; ir != nnl[iproj].nr; ++ir) {
499-
EXPECT_DOUBLE_EQ(tmp.r_radial[ir], nnl[iproj].r_radial[ir]);
500-
EXPECT_DOUBLE_EQ(tmp.rab[ir], nnl[iproj].rab[ir]);
501-
EXPECT_DOUBLE_EQ(tmp.beta_r[ir], nnl[iproj].beta_r[ir]);
507+
for (int ir = 0; ir != nnl[iproj].getNr(); ++ir) {
508+
EXPECT_DOUBLE_EQ(tmp.getRadial(ir), nnl[iproj].getRadial(ir));
509+
EXPECT_DOUBLE_EQ(rab_of(tmp)[ir], rab_of(nnl[iproj])[ir]);
510+
EXPECT_DOUBLE_EQ(tmp.getBeta_r(ir), nnl[iproj].getBeta_r(ir));
502511
}
503512

504-
for (int ik = 0; ik != nnl[iproj].nk; ++ik) {
505-
EXPECT_DOUBLE_EQ(tmp.k_radial[ik], nnl[iproj].k_radial[ik]);
506-
EXPECT_DOUBLE_EQ(tmp.beta_k[ik], nnl[iproj].beta_k[ik]);
513+
for (int ik = 0; ik != nnl[iproj].getNk(); ++ik) {
514+
EXPECT_DOUBLE_EQ(tmp.getKpoint(ik), nnl[iproj].getKpoint(ik));
515+
EXPECT_DOUBLE_EQ(tmp.getBeta_k(ik), nnl[iproj].getBeta_k(ik));
507516
}
508517
}
509518

@@ -535,7 +544,7 @@ TEST_F(NumericalNonlocalLmTest, R2K2RConsistency) {
535544
for (size_t iproj = 0; iproj != nnl.size(); ++iproj) {
536545
Numerical_Nonlocal_Lm tmp;
537546
tmp = nnl[iproj];
538-
this->change_k(tmp, ecut, tmp.dk);
547+
this->change_k(tmp, ecut, tmp.getDk());
539548
EXPECT_LT(err_r2k2r(tmp), 1e-6);
540549
}
541550
}
@@ -554,11 +563,11 @@ TEST_F(NumericalNonlocalLmTest, R2K2RConsistencyMany) {
554563
for (size_t ie = 0; ie != ecut_list.size(); ++ie) {
555564
Numerical_Nonlocal_Lm tmp;
556565
tmp = nnl[iproj];
557-
this->change_k(tmp, ecut_list[ie], tmp.dk);
566+
this->change_k(tmp, ecut_list[ie], tmp.getDk());
558567
double err = err_r2k2r(tmp);
559568
std::cout << "proj = " << iproj
560569
<< " ecut = " << std::setw(8) << ecut_list[ie]
561-
<< " dk = " << std::setw(6) << tmp.dk
570+
<< " dk = " << std::setw(6) << tmp.getDk()
562571
<< " error = " << std::setw(10) << err
563572
<< std::endl;
564573
}
@@ -584,7 +593,7 @@ TEST_F(NumericalNonlocalLmTest, R2K2RConsistencyMany) {
584593
<< " ecut = " << std::setw(6) << ecut
585594
<< " dk = " << std::setw(6) << dk_list[idk]
586595
<< " error = " << std::setw(12) << err
587-
<< " nk = " << std::setw(5) << tmp.nr
596+
<< " nk = " << std::setw(5) << tmp.getNr()
588597
<< std::endl;
589598
}
590599
std::cout << std::endl;
@@ -597,48 +606,48 @@ TEST_F(NumericalNonlocalLmTest, FreeAndRenew) {
597606

598607
this->init();
599608

600-
EXPECT_NE(nnl[0].r_radial, nullptr);
601-
EXPECT_NE(nnl[0].rab, nullptr);
602-
EXPECT_NE(nnl[0].beta_r, nullptr);
609+
EXPECT_NE(nnl[0].getRadial(), nullptr);
610+
EXPECT_NE(rab_of(nnl[0]), nullptr);
611+
EXPECT_NE(nnl[0].getBeta_r(), nullptr);
603612
EXPECT_NE(nnl[0].beta_uniform, nullptr);
604613
EXPECT_NE(nnl[0].dbeta_uniform, nullptr);
605-
EXPECT_NE(nnl[0].k_radial, nullptr);
606-
EXPECT_NE(nnl[0].beta_k, nullptr);
614+
EXPECT_NE(nnl[0].getKpoint(), nullptr);
615+
EXPECT_NE(nnl[0].getBeta_k(), nullptr);
607616

608-
nnl[0].freemem();
617+
freemem(nnl[0]);
609618

610-
EXPECT_EQ(nnl[0].r_radial, nullptr);
611-
EXPECT_EQ(nnl[0].rab, nullptr);
612-
EXPECT_EQ(nnl[0].beta_r, nullptr);
619+
EXPECT_EQ(nnl[0].getRadial(), nullptr);
620+
EXPECT_EQ(rab_of(nnl[0]), nullptr);
621+
EXPECT_EQ(nnl[0].getBeta_r(), nullptr);
613622
EXPECT_EQ(nnl[0].beta_uniform, nullptr);
614623
EXPECT_EQ(nnl[0].dbeta_uniform, nullptr);
615-
EXPECT_EQ(nnl[0].k_radial, nullptr);
616-
EXPECT_EQ(nnl[0].beta_k, nullptr);
624+
EXPECT_EQ(nnl[0].getKpoint(), nullptr);
625+
EXPECT_EQ(nnl[0].getBeta_k(), nullptr);
617626

618-
nnl[0].renew();
627+
renew(nnl[0]);
619628

620-
ASSERT_NE(nnl[0].r_radial, nullptr);
621-
ASSERT_NE(nnl[0].rab, nullptr);
622-
ASSERT_NE(nnl[0].beta_r, nullptr);
629+
ASSERT_NE(nnl[0].getRadial(), nullptr);
630+
ASSERT_NE(rab_of(nnl[0]), nullptr);
631+
ASSERT_NE(nnl[0].getBeta_r(), nullptr);
623632
ASSERT_NE(nnl[0].beta_uniform, nullptr);
624633
ASSERT_NE(nnl[0].dbeta_uniform, nullptr);
625-
ASSERT_NE(nnl[0].k_radial, nullptr);
626-
ASSERT_NE(nnl[0].beta_k, nullptr);
634+
ASSERT_NE(nnl[0].getKpoint(), nullptr);
635+
ASSERT_NE(nnl[0].getBeta_k(), nullptr);
627636

628-
for (int ir = 0; ir != nnl[0].nr; ++ir) {
629-
EXPECT_DOUBLE_EQ(nnl[0].r_radial[ir], 0.0);
630-
EXPECT_DOUBLE_EQ(nnl[0].rab[ir], 0.0);
631-
EXPECT_DOUBLE_EQ(nnl[0].beta_r[ir], 0.0);
637+
for (int ir = 0; ir != nnl[0].getNr(); ++ir) {
638+
EXPECT_DOUBLE_EQ(nnl[0].getRadial(ir), 0.0);
639+
EXPECT_DOUBLE_EQ(rab_of(nnl[0])[ir], 0.0);
640+
EXPECT_DOUBLE_EQ(nnl[0].getBeta_r(ir), 0.0);
632641
}
633642

634643
for (int ir = 0; ir != nnl[0].nr_uniform; ++ir) {
635644
EXPECT_DOUBLE_EQ(nnl[0].beta_uniform[ir], 0.0);
636645
EXPECT_DOUBLE_EQ(nnl[0].dbeta_uniform[ir], 0.0);
637646
}
638-
639-
for (int ik = 0; ik != nnl[0].nk; ++ik) {
640-
EXPECT_DOUBLE_EQ(nnl[0].k_radial[ik], 0.0);
641-
EXPECT_DOUBLE_EQ(nnl[0].beta_k[ik], 0.0);
647+
648+
for (int ik = 0; ik != nnl[0].getNk(); ++ik) {
649+
EXPECT_DOUBLE_EQ(nnl[0].getKpoint(ik), 0.0);
650+
EXPECT_DOUBLE_EQ(nnl[0].getBeta_k(ik), 0.0);
642651
}
643652
}
644653

@@ -709,10 +718,10 @@ TEST_F(NumericalNonlocalLmTest, BetaSave) {
709718
std::string betak_fname = dir+"/O-" + orb[i] + "-proj-k.dat";
710719
std::string betaru_fname = dir+"/O-" + orb[i] + "-proj-ru.dat";
711720

712-
EXPECT_EQ(true, this->check_file_match(nnl[i].nr,
713-
nnl[i].r_radial, nnl[i].beta_r, tol, betar_fname));
714-
EXPECT_EQ(true, this->check_file_match(nnl[i].nk,
715-
nnl[i].k_radial, nnl[i].beta_k, tol, betak_fname));
721+
EXPECT_EQ(true, this->check_file_match(nnl[i].getNr(),
722+
nnl[i].getRadial(), nnl[i].getBeta_r(), tol, betar_fname));
723+
EXPECT_EQ(true, this->check_file_match(nnl[i].getNk(),
724+
nnl[i].getKpoint(), nnl[i].getBeta_k(), tol, betak_fname));
716725

717726
double* r_uniform_mesh = new double[nnl[i].nr_uniform];
718727
for (int ir = 0; ir != nnl[i].nr_uniform; ++ir) {

0 commit comments

Comments
 (0)