0

My function looks like this:

def accum(s):
    a = []
    for i in s:
        b = s.index(i)
        a.append(i * (b+1))
    x = "-".join(a)
    return x.title()

with the expected input of:

'abcd'

the output should be and is:

'A-Bb-Ccc-Dddd' 

but if the input has a recurring character:

'abccba'

it returns:

'A-Bb-Ccc-Ccc-Bb-A'

instead of:

'A-Bb-Ccc-Cccc-Bbbbb-Aaaaaa'

how can I fix this?

Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343

3 Answers3

1

Don't use str.index(), it'll return the first match. Since c and b and a appear early in the string you get 2, 1 and 0 back regardless of the position of the current letter.

Use the enumerate() function to give you position counter instead:

for i, letter in enumerate(s, 1):
    a.append(i * letter)

The second argument is the starting value; setting this to 1 means you can avoid having to + 1 later on. See What does enumerate mean? if you need more details on what enumerate() does.

You can use a list comprehension here rather than use list.append() calls:

def accum(s):
    a = [i * letter for i, letter in enumerate(s, 1)]
    x = "-".join(a)
    return x.title()

which could, at a pinch, be turned into a one-liner:

def accum(s):
    a = '-'.join([i * c for i, c in enumerate(s, 1)]).title()
Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
  • I was unaware of `enumerate(_, starting_index)`. I'm not certain if using it for this is more or less Pythonic than incrementing a zero-based index. It's definitely a cool solution though. – orlp Oct 04 '16 at 14:45
  • @orlp: why would it be less Pythonic to use a built-in iterator that adds an index counter, eliminating a lot of unnecessary manual counting? – Martijn Pieters Oct 04 '16 at 14:47
  • Because, in my mind, the second you write `c * i`, `i` is not an index anymore. In this case it's rather coincidental that the formula of the number of characters is 1 + the zero based index, and I think it's not a bad thing to emphasize that this is indeed a formula, by writing `(i + 1)`. But I'm not really certain either way. P.S.: `join([i * c for i, c in enumerate(s, 1)])` hurts my eyes, those superfluous `[]`. – orlp Oct 04 '16 at 14:51
  • @orlp: `str.join()` *requires* a list; it'll convert anything else to a list. That makes a generator expression slower here, so I always use a list comp. It's an exception, otherwise I'd agree on the redundancy. See [List comprehension without \[ \] in Python](//stackoverflow.com/a/9061024) – Martijn Pieters Oct 04 '16 at 14:57
  • I wasn't aware of that, regardless I wouldn't write it with superfluous parenthesis unless that code is in a profiled bottleneck. But I see where you're coming from. – orlp Oct 04 '16 at 15:00
  • @MartijnPieters Do you know why it requires a list? Why would it convert a tuple or string or any other sequence to a list for this? – Stefan Pochmann Oct 04 '16 at 15:03
  • @StefanPochmann: it requires a random-accessible sequence, but in practice this means it'll create a list from the input. It needs to do 2 passes: first to calculate the total length of the target string, then to copy the string contents of each element. – Martijn Pieters Oct 04 '16 at 15:12
  • @StefanPochmann: ah, correction. Either I misremember or this was updated since; a tuple won't be converted to a list here. [`PySequence_Fast()`](https://docs.python.org/3/c-api/sequence.html#c.PySequence_Fast) is used for this. – Martijn Pieters Oct 04 '16 at 15:15
  • @MartijnPieters I just traced it there as well and was about to show you :-). But now I'm confused why `PySequence_Fast` is documented with "Return the sequence o as ..." and "If the object is not a sequence, raises TypeError". So it already requires a sequence? Then why does it work with a generator at all? – Stefan Pochmann Oct 04 '16 at 15:20
  • @StefanPochmann: Feel free to submit an issue in the Python tracker to have that improved. It accepts any *iterable* (`PyObject_GetIter()` is used, equivalent of `iter()` in Python code). If the object is already a list or tuple, that's returned, if `iterator = PyObject_GetIter()` fails, an exception is raised, otherwise `list(iterator)` is returned. – Martijn Pieters Oct 04 '16 at 15:23
1

This is because s.index(a) returns the first index of the character. You can use enumerate to pair elements to their indices:

Here is a Pythonic solution:

def accum(s):
    return "-".join(c*(i+1) for i, c in enumerate(s)).title()
orlp
  • 112,504
  • 36
  • 218
  • 315
0

simple:

def accum(s):
    a = []
    for i in range(len(s)):
        a.append(s[i]*(i+1))
    x = "-".join(a)
    return x.title()
Howardyan
  • 667
  • 1
  • 6
  • 15