Closed GoogleCodeExporter closed 9 years ago
La cojo, ya veremos como me sale
Original comment by jwl...@gmail.com
on 13 Nov 2010 at 5:34
Ruta de Task Description cambiada a
http://quimeraengine.googlecode.com/files/QVector3.pdf.
Original comment by Lince3D@gmail.com
on 15 Nov 2010 at 8:12
Original comment by jwl...@gmail.com
on 20 Nov 2010 at 10:59
Resultado de revisión:
- Inclusiones <memory> y <math.h> ¿son realmente necesarias?
- "This class implements three components Vector funcionality.": No sé hasta
qué punto es clarificador poner Vector con mayúscula. A funcionality le falta
la t.
- "QVector3() : QBaseVector3(0.0f, 0.0f, 0.0f) { }": Debería llamar a
constructor por defecto de QBaseVector3, y ser éste quien lo ponga todo a
cero. Debería ser inline.
- "inline explicit QVector3(const QVector3 &v) { memcpy(this, &v,
sizeof(QVector3)); }": No era necesario implementar el constructor de copia.
- "QVector3(const QBaseVector3 &v) : QBaseVector3(v.x, v.y, v.z) { }": Debería
llamar al constructor de copia de QBaseVector3. Debería ser inline.
- "QVector3(const float_q &fValueX, const float_q &fValueY, const float_q
&fValueZ) : QBaseVector3(fValueX, fValueY, fValueZ) { }": Debería ser inline.
- "explicit QVector3(const float_q &fValue) : QBaseVector3(fValue, fValue,
fValue) { }": Debería llamar al constructor de QBaseVector3 que aceptase un
float_q. Debería ser inline.
- "inline explicit QVector3(const float_q *pValue)": Debería llamar al
constructor de QBaseVector3 que aceptase el puntero.
- "virtual ~QVector3();": El destructor no hay que implementarlo. No deben
usase virtuals en esta ocasión.
- "/// Product by a scalar: all components are multiplied by the float value
provided.": Dado que usamos un tipo configurable de precisión, no debemos
poner float, que es un tipo concreto, sino "floating point type", o "real
number".
- Los bloques de documentación deberían estar alineados al código que
documentan.
- "if (v.x > x - EPSILON_Q && v.x < x + ...": Se aconseja el uso de paréntesis
para asegurar el orden de precedencia y mejorar la legibilidad.
- En la misma sentencia que el anterior punto (operator== y !=), es posible
ponerla con un único return.
- En general: Salvo llaves vacías {}, el resto debe ir una llave en una línea
propia.
- "friend QVector3 operator * (const float_q &fValue, QVector3 &v);": ¿Por
qué el parámetro v no es const?
- "{ x += v.x; y += v.y; z += v.z; return (*this); }": Cada sentencia en una
línea, mejora la legibilidad aunque quede menos "bonito".
- "(*this)": No está mal, pero los paréntesis no son necesarios.
- El producto vectorial implementado da como resultado un vector perpendicular
que sigue la regla de la mano derecha, mientras que el motor sigue el sistema
de la mano izquierda. El vector resultante debería ser el opuesto.
- "x = x*fFactor + v.x*(1-fFactor); y = y*fFactor + v.y*(1-fFactor); x =
z*fFactor + v.z*(1-fFactor);": En el método Lerp, en la última suma, es z y
no x (los copypastes los carga el diablo ;)).
- "A float value wich represents how close is...": Falta una H en which.
Original comment by Lince3D@gmail.com
on 21 Nov 2010 at 12:18
Original comment by jwl...@gmail.com
on 21 Nov 2010 at 9:02
Resultado de la 2ª review:
-"Makes a Linear Interpolation betwen current": Falta una e en between.
-CrossProduct: La idea era guardar el resultado en el propio vector, de todas
formas ese déjalo hecho (en la extension se pide) y hazle la versión que
guarda en sí mismo.
-En el cpp hay que borrar los comentarios //usings y // Static attributes
inicialization (la propia sección en la que está es únicamente para eso,
resulta redundante).
-Sé que aquí no me he explicado bien, pero no hacía falta crear el
constructor de copia en QBaseVector3, ya que C++ proporciona uno por defecto,
igual que hace con el operator=, el constructor y el destructor.
-Pon un [TODO] Entre las llaves de aquellos métodos que aún no estén
implementados.
Venga, que este va a quedar fino fino :) .
Original comment by Lince3D@gmail.com
on 22 Nov 2010 at 7:53
Original comment by jwl...@gmail.com
on 22 Nov 2010 at 9:36
Original issue reported on code.google.com by
Lince3D@gmail.com
on 12 Nov 2010 at 9:02