Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Quite a specific question can the below code be made more clearer / pythonic in any way.

Essentially based on the condition of the string x((len(x) / 3) - 1), I use this to extract out parts of the string.

def split_string(x):
    splitted_string = [x[counter*3:(counter*3)+3] for counter in range ((len(x) /3) - 1)]
    return splitted_string

>>> split_string('ABCDEFGHI')
['ABC', 'DEF']
share|improve this question

For integer division, use the // operator. In Python 3, / will do floating-point division.

The task could be more succinctly accomplished using re.findall().

Also, the goal of the function is not obvious. Pythonic code would include a docstring explaining its purpose.

import re

def split_string(s):
    Splits a string into a list, with each element consisting of
    three characters.  Any incomplete group at the end is discarded,
    and then the last complete group is also discarded.
    return re.findall(r'.{3}', s)[:-1]
share|improve this answer

I always try to evaluate my function and see if there's anything else that could enhance it.

You're already evenly splitting these strings, so why not add a default parameter, size to control the section size. You also don't take the last cut (I'm guessing to avoid index error), you can add that as a default parameter as well.

Also, just return the comprehension

def split_string(word, size = 3, trim = 1):
    return [word[ii*size:(ii+1)*size] for ii in range(len(word)/size - trim )]

>>> split_string('ABCDEFGHI')
['ABC', 'DEF']
>>> split_string('ABCDEFGHI',4)
['ABCD', 'EFGH']
>>> split_string('ABCDEFGHI',3,0)
['ABC', 'DEF','GHI']
>>> split_string('ABCDEFGHI',3,2)
share|improve this answer
Menaningful parameter names would be even better :) – Morwenn Apr 10 '14 at 21:56
@Morwenn I agree,.. I was just lazy :p – flkes Apr 10 '14 at 21:57

The range command has a step argument.

def split_string(x):
    return [x[i:i+3] for i in range(0, len(x) - 5, 3)]

>>> split_string('ABCDEFGHI')
['ABC', 'DEF']

I also used i instead of counter since it's shorter and i is the standard way of expressing an array index in most langauges.

share|improve this answer
That's a nice idea, but the behaviour is quite different from the original code. – 200_success Apr 10 '14 at 21:43
It was just missing the -1 on the end of the range. – David Harkness Apr 11 '14 at 2:29
@DavidHarkness It's still not equivalent to the original. – 200_success Apr 11 '14 at 7:39
@200_success I corrected my correction. – David Harkness Apr 11 '14 at 8:09
@DavidHarkness There was still an off-by-one error, which I've fixed in Rev 4. – 200_success Apr 11 '14 at 9:23

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.