[webkit-reviews] review denied: [Bug 34548] audio engine: add Vector3 class : [Attachment 48160] address Sam's style comments

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Chris Rogers <crogers at google.com>'s
request for review:
Bug 34548: audio engine: add Vector3 class
https://bugs.webkit.org/show_bug.cgi?id=34548

Attachment 48160: address Sam's style comments
https://bugs.webkit.org/attachment.cgi?id=48160&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
Looks good.

> +    Vector3(float* p)

This should be const float*, or actually:

    Vector(const float p[3])

Which documents what the pointer should point to, but is otherwise equivalent.

> +    Vector3(double* p)

Ditto. const double[3] would be best.

> +    // Explicit copy-constructor
> +    Vector3(const Vector3& v)

Why? The compiler will generate exactly this, so why define it explicitly? It's
typically considered better style to omit this.

> +    double dot(const Vector3&) const;
> +
> +    Vector3 cross(const Vector3&) const;
> +
> +    double distance(const Vector3&) const;

Do we need these members? Why not just have plain old functions? Is code that
uses the member function syntax clearer? Could you show me an example?

> +    friend double dot(const Vector3&, const Vector3&);
> +    friend Vector3 cross(const Vector3&, const Vector3&);
> +    friend Vector3 operator*(double, const Vector3&);
> +    friend Vector3 operator+(const Vector3&, const Vector3&);
> +    friend Vector3 operator-(const Vector3&, const Vector3&);

Why do these need to be friends? They should use the public inline x(), y() and
z() functions and so won't need to be friends.

review- for now, but seems generally good