4
\$\begingroup\$

I made a word counter. It works as long as there aren't any lone punctuation marks. How could it be improved? (Could it be made simpler? Are the comments detailed/clear enough? etc.) I know it's simple, but I don't want to develop bad habits early on.

#prompt user to input a sentence
toCount = raw_input("--- ")

#function that counts words of inputted sentence
def countWords(theString):

    stringLength = len(theString)

    #index for each character in string
    counter1 = 0

    #stores word count
    accumulator = 0

    #go through each character in input text. if a character is a space
    #with no preceding spaces, add 1 to word count.
    while counter1 < stringLength:
        stringChar = theString[counter1]
        if stringChar == ' ' and counter1 > 0 and theString[counter1 - 1] != ' ':
            accumulator = accumulator + 1
            counter1 = counter1 + 1
        else:
            counter1 = counter1 + 1

    #if last character in input text is not a space, add 1 to word count
    if theString[counter1 - 1] != ' ':
        accumulator = accumulator + 1

    output = "You typed " + str(accumulator) + " words."

    return output

print countWords(toCount)
\$\endgroup\$
6
\$\begingroup\$

Here's one way to simplify it:

inp = raw_input("--- ")

print len(inp.split())

I'm not sure if you were unaware of this or if you were .

Before I go all out, maybe you could comment or leave an edit on your question letting us know if you meant to reinvent things here.

\$\endgroup\$
3
  • \$\begingroup\$ I was not aware of it. I have never seen the inp.split() method and I have no idea what wheel I would be reinventing. \$\endgroup\$
    – waggysax
    Jun 15 '14 at 0:08
  • \$\begingroup\$ ...but I figured it out. Thank you. \$\endgroup\$
    – waggysax
    Jun 15 '14 at 0:23
  • 2
    \$\begingroup\$ @user3740797 Here's more on that split method. The term "reinventing-the-wheel" simply means building your own version of something that already exists. \$\endgroup\$
    – Alex L
    Jun 15 '14 at 2:11
6
\$\begingroup\$

As Alex L points out, your approach could be significantly simplified. However, in terms of the actual code you've written, there are a few definite improvements you could make.


You can use += and smarter scoping to simplify your loop. This:

if stringChar == ' ' and counter1 > 0 and theString[counter1 - 1] != ' ':
    accumulator = accumulator + 1
    counter1 = counter1 + 1
else:
    counter1 = counter1 + 1

simplifies to:

if stringChar == ' ' and counter1 > 0 and theString[counter1 - 1] != ' ':
    accumulator += 1
counter1 += 1

As a rule, fewer lines of code is less potential for error.


More generally, it is not a good idea to use a while loop where a for loop could be used; the former is generally more error-prone. Compare:

index = 0
while index < len(things):
    item = things[index]
    ...
    index += 1

with:

for index in range(len(things)):
    item = things[index]
    ...

Better still, rather than manually using an index, iterate over the iterable directly:

for item in things:
    ...

In your case, as you want to look at pairs of consecutive items, you can use zip and a slice:

for first_item, second_item in zip(things, things[1:]):
    ...
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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