Author lemburg
Recipients ezio.melotti, flox, lemburg, pitrou
Date 2010-01-04.13:02:59
SpamBayes Score 1.17684e-14
Marked as misclassified No
Message-id <1262610183.83.0.772564987223.issue7622@psf.upfronthosting.co.za>
In-reply-to
Content
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)
  -{
History
Date User Action Args
2010-01-04 13:03:04lemburgsetrecipients: + lemburg, pitrou, ezio.melotti, flox
2010-01-04 13:03:03lemburgsetmessageid: <1262610183.83.0.772564987223.issue7622@psf.upfronthosting.co.za>
2010-01-04 13:03:02lemburglinkissue7622 messages
2010-01-04 13:03:00lemburgcreate