Marc-Andre Lemburg <m...@egenix.com> added the comment:

A few comments on coding style:

 * please keep the existing argument formats as they are, e.g.

    count = countstring(self_s, self_len,
                        from_s, from_len,
                        0, self_len, FORWARD, maxcount);

   or

  /* helper macro to fixup start/end slice values */
  -#define FIX_START_END(obj)                      \
  -    if (start < 0)                              \
  -        start += (obj)->length;                 \
  -    if (start < 0)                              \
  -        start = 0;                              \
  -    if (end > (obj)->length)                    \
  -        end = (obj)->length;                    \
  -    if (end < 0)                                \
  -        end += (obj)->length;                   \
  -    if (end < 0)                                \
  -        end = 0;
  +#define ADJUST_INDICES(start, end, len)                         \
  +    if (end > len) end = len;                                   \
  +    else if (end < 0) { end += len; if (end < 0) end = 0; }     \
  +    if (start < 0) { start += len; if (start < 0) start = 0; }



   and use similar formatting for the replacement function
   calls/macros

 * make sure that the name of a symbol matches the value, e.g.

   #define LONG_BITMASK (LONG_BIT-1)
   #define BLOOM(mask, ch) ((mask & (1 << ((ch) & LONG_BITMASK))))

   LONG_BITMASK has a value of 0x1f (31) - that's a single byte, not
   a long value. In this case, 0x1f is an implementation detail of
   the simplified Bloom filter used for set membership tests in the
   Unicode implementation.

   When adjusting the value to be platform dependent, please check
   that the implementation does work for platforms that have
   more than 31 bits available for (signed) longs.

   Note that you don't need to expose that value separately if
   you stick to using BLOOM() directly.

 * use BLOOM() macro in fastsearch.c

 * when declaring variables with initial values, keep these on separate lines, 
e.g. don't use this style:

    Py_ssize_t i, j, count=0;
    PyObject *list = PyList_New(PREALLOC_SIZE(maxcount)), *sub;

   instead, write:

    Py_ssize_t i, j;
    Py_ssize_t count=0;
    PyObject *list = PyList_New(PREALLOC_SIZE(maxcount))
    PyObject *sub;

 * always place variable declarations at the top of a function, not into the 
function body:

  +stringlib_split(
  +    PyObject* str_obj, const STRINGLIB_CHAR* str, Py_ssize_t str_len,
  +    const STRINGLIB_CHAR* sep, Py_ssize_t sep_len, Py_ssize_t  maxcount
  +    )
  +{
  +    if (sep_len == 0) {
  +        PyErr_SetString(PyExc_ValueError, "empty separator");
  +        return NULL;
  +    }
  +    else if (sep_len == 1)
  +        return stringlib_split_char(str_obj, str, str_len, sep[0],   
maxcount);
  +
  +    Py_ssize_t i, j, pos, count=0;
  +    PyObject *list = PyList_New(PREALLOC_SIZE(maxcount)), *sub;
  +    if (list == NULL)
  +        return NULL;
   
 * function declarations should not put parameters on new lines:

  +stringlib_splitlines(
  +    PyObject* str_obj, const STRINGLIB_CHAR* str, Py_ssize_t str_len,
  +    int keepends
  +    )
  +{

 instead use this style:

  -static
  -PyObject *rsplit_substring(PyUnicodeObject *self,
  -                           PyObject *list,
  -                           PyUnicodeObject *substring,
  -                           Py_ssize_t maxcount)
  -{

----------
components: +Unicode
nosy: +lemburg

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue7622>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to