Skip to content

Commit

Permalink
Normalize constructor parameter names (#1427)
Browse files Browse the repository at this point in the history
* Normalize constructor parameter names

In order to avoid confusion between constructor parameter names and
member variables, we either used alternate names for the parameters or
prefixed the parameter names with an underscore. For example:

    class foo {
      public:
        foo(int _i, bool semaphore, short bar) : i(_i), flag(semaphore)
        {
            baz = bar;
        }
       private:
         int i;
         bool flag;
         short baz;
    };

However, this is unnecessary. Constructor initializer lists are
unambiguous, as the assigned item always refers to the class member, and
the assigned value is always from the parameter (unless prefixed with
"this->").

Inside the constructor body, parameter names override member variables,
and "this->" can be used to indicate the class member in the case of
name collision.

Thus, the above code can be rewritten as this:

    class foo {
      public:
        foo(int i, bool flag, short baz) : i(i), flag(flag)
        {
            this->baz = baz;
        }
       private:
         int i;
         bool flag;
         short baz;
    };

I've taken advantage of this to clarify names used in constructors
throughout the codebase. Mostly for constructor parameter names,
sometimes for member variable names.

* Refine names from review feedback
  • Loading branch information
hollasch committed Mar 12, 2024
1 parent 45383aa commit ace8e94
Show file tree
Hide file tree
Showing 24 changed files with 201 additions and 189 deletions.
37 changes: 20 additions & 17 deletions books/RayTracingInOneWeekend.html
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,7 @@

class sphere : public hittable {
public:
sphere(const point3& _center, double _radius) : center(_center), radius(_radius) {}
sphere(const point3& center, double radius) : center(center), radius(radius) {}

bool hit(const ray& r, double ray_tmin, double ray_tmax, hit_record& rec) const override {
vec3 oc = center - r.origin();
Expand Down Expand Up @@ -1551,7 +1551,7 @@

interval() : min(+infinity), max(-infinity) {} // Default interval is empty

interval(double _min, double _max) : min(_min), max(_max) {}
interval(double min, double max) : min(min), max(max) {}

double size() const {
return max - min;
Expand Down Expand Up @@ -2794,8 +2794,8 @@
class sphere : public hittable {
public:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
sphere(const point3& _center, double _radius, shared_ptr<material> _material)
: center(_center), radius(_radius), mat(_material) {}
sphere(const point3& center, double radius, shared_ptr<material> mat)
: center(center), radius(radius), mat(mat) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++

bool hit(const ray& r, interval ray_t, hit_record& rec) const override {
Expand Down Expand Up @@ -2844,7 +2844,7 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ Highlight
class lambertian : public material {
public:
lambertian(const color& a) : albedo(a) {}
lambertian(const color& albedo) : albedo(albedo) {}

bool scatter(const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered)
const override {
Expand Down Expand Up @@ -2896,7 +2896,7 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class lambertian : public material {
public:
lambertian(const color& a) : albedo(a) {}
lambertian(const color& albedo) : albedo(albedo) {}

bool scatter(const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered)
const override {
Expand Down Expand Up @@ -2964,7 +2964,7 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ Highlight
class metal : public material {
public:
metal(const color& a) : albedo(a) {}
metal(const color& albedo) : albedo(albedo) {}

bool scatter(const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered)
const override {
Expand Down Expand Up @@ -3099,7 +3099,7 @@
class metal : public material {
public:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
metal(const color& a, double f) : albedo(a), fuzz(f < 1 ? f : 1) {}
metal(const color& albedo, double fuzz) : albedo(albedo), fuzz(fuzz < 1 ? fuzz : 1) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++

bool scatter(const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered)
Expand Down Expand Up @@ -3252,12 +3252,12 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ Highlight
class dielectric : public material {
public:
dielectric(double index_of_refraction) : ir(index_of_refraction) {}
dielectric(double ref_index) : ref_index(ref_index) {}

bool scatter(const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered)
const override {
attenuation = color(1.0, 1.0, 1.0);
double refraction_ratio = rec.front_face ? (1.0/ir) : ir;
double refraction_ratio = rec.front_face ? (1.0/ref_index) : ref_index;

vec3 unit_direction = unit_vector(r_in.direction());
vec3 refracted = refract(unit_direction, rec.normal, refraction_ratio);
Expand All @@ -3267,7 +3267,8 @@
}

private:
double ir; // Index of Refraction
double ref_index; // Refractive index in vacuum or air, or the ratio of the material's
// refractive index over the refractive index of the enclosing media
};
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[Listing [dielectric-always-refract]: <kbd>[material.h]</kbd>
Expand Down Expand Up @@ -3365,12 +3366,12 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class dielectric : public material {
public:
dielectric(double index_of_refraction) : ir(index_of_refraction) {}
dielectric(double ref_index) : ref_index(ref_index) {}

bool scatter(const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered)
const override {
attenuation = color(1.0, 1.0, 1.0);
double refraction_ratio = rec.front_face ? (1.0/ir) : ir;
double refraction_ratio = rec.front_face ? (1.0/ref_index) : ref_index;

vec3 unit_direction = unit_vector(r_in.direction());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
Expand All @@ -3391,7 +3392,8 @@
}

private:
double ir; // Index of Refraction
double ref_index; // Refractive index in vacuum or air, or the ratio of the material's
// refractive index over the refractive index of the enclosing media
};
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[Listing [dielectric-with-refraction]: <kbd>[material.h]</kbd>
Expand Down Expand Up @@ -3433,12 +3435,12 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class dielectric : public material {
public:
dielectric(double index_of_refraction) : ir(index_of_refraction) {}
dielectric(double ref_index) : ref_index(ref_index) {}

bool scatter(const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered)
const override {
attenuation = color(1.0, 1.0, 1.0);
double refraction_ratio = rec.front_face ? (1.0/ir) : ir;
double refraction_ratio = rec.front_face ? (1.0/ref_index) : ref_index;

vec3 unit_direction = unit_vector(r_in.direction());
double cos_theta = fmin(dot(-unit_direction, rec.normal), 1.0);
Expand All @@ -3459,7 +3461,8 @@
}

private:
double ir; // Index of Refraction
double ref_index; // Refractive index in vacuum or air, or the ratio of the material's
// refractive index over the refractive index of the enclosing media


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
Expand Down
Loading

0 comments on commit ace8e94

Please sign in to comment.