[Bf-codereview] Mathutils n-dimension patch (issue 5449088)

[ideasman42 at gmail.com]

Reviewers: bf-codereview_blender.org,

Message:
comments inline, mostly api usage.


http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils.c
File source/blender/python/mathutils/mathutils.c (right):

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils.c#newcode163
source/blender/python/mathutils/mathutils.c:163: {
array_min is not getting checked in this case

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c
File source/blender/python/mathutils/mathutils_Vector.c (right):

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode112
source/blender/python/mathutils/mathutils_Vector.c:112: if
(!PyArg_ParseTuple(args, "i|f", &size, &fill)) {
to avoid hand writing exceptions for PyArg_ParseTuple you can replace
"i|f" with "i|f":Vector.Fill", then just return NULL.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode345
source/blender/python/mathutils/mathutils_Vector.c:345: static PyObject
*Vector_resize(VectorObject *self, PyObject *args)
(picky) for functions which recieve a single argument you can use
METH_O, in the methoddef and then take the PyObject and get its an int
(not deal with args tuples). enough examples of this in the code.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode381
source/blender/python/mathutils/mathutils_Vector.c:381: self->vec[i]=
0.0f;
(picky) why not use range_vn_fl fill function here? - just need to
offset the array.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode694
source/blender/python/mathutils/mathutils_Vector.c:694: static PyObject
*Vector_dot(VectorObject *self, PyObject *value)
(picky), this is an example of where you can use cleanup...

697 » PyObject *ret= NULL;
....
  if (some error)
    goto cleanup;

cleanup:
     PyMem_Free(tvec);
     return ret;
----

since its simple func its not going to confuse anyway, but would do it
this way all the same.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode885
source/blender/python/mathutils/mathutils_Vector.c:885: if
(mathutils_array_parse_alloc(&tvec, size, value, "Vector.lerp(other),
invalid 'other' arg") == -1) {
again, suggest using a `cleanup` goto at the end.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode1128
source/blender/python/mathutils/mathutils_Vector.c:1128: vec=
PyMem_Malloc(vec1->size * sizeof(float));
no check on fail here.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode1192
source/blender/python/mathutils/mathutils_Vector.c:1192: vec=
PyMem_Malloc(vec1->size * sizeof(float));
also no check on fail.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode1279
source/blender/python/mathutils/mathutils_Vector.c:1279: tvec=
PyMem_Malloc(vec->size * sizeof(float));
no check on fail.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode1480
source/blender/python/mathutils/mathutils_Vector.c:1480: vec=
PyMem_Malloc(vec1->size * sizeof(float));
no check on fail.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode2422
source/blender/python/mathutils/mathutils_Vector.c:2422: /*{"func",
(PyCFunction) Vector_func, METH_VARARGS, NULL},*/
think the to_2/3/4 d funcs can stay for now, at least not to break the
api, see how resized is more flexible tho.

http://codereview.appspot.com/5449088/diff/1/source/blender/python/mathutils/mathutils_Vector.c#newcode2598
source/blender/python/mathutils/mathutils_Vector.c:2598: static PyObject
*Vector_CreatePyObject_alloc(float *vec, const int size, const int type,
PyTypeObject *base_type)
no need for type argument



Please review this at http://codereview.appspot.com/5449088/

Affected files:
   M     source/blender/blenkernel/intern/DerivedMesh.c
   M     source/blender/blenlib/BLI_math_vector.h
   M     source/blender/blenlib/intern/math_vector.c
   M     source/blender/python/mathutils/mathutils.c
   M     source/blender/python/mathutils/mathutils.h
   M     source/blender/python/mathutils/mathutils_Vector.c
   M     source/blender/python/mathutils/mathutils_Vector.h