11
\$\begingroup\$

I want to split a song into multiple parts, given the song duration and number of parts.

My code achieves that, but it feels a little "stupid" and I would like to learn a more sophisticated - and shorter - way. Particularly, I feel that the marker variable is a little overkill. I would welcome any suggestions.

song_duration = 20 # these two 
num_of_parts = 4   # are given

part_duration = song_duration / num_of_parts
parts = []
marker = 0

for _ in range(num_of_parts):
    part = [marker, marker + part_duration]
    marker += part_duration
    parts.append(part)

print(parts)

# parts is : [[0, 5.0], [5.0, 10.0], [10.0, 15.0], [15.0, 20.0]]
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to CodeReview ! From the output sample you've provided, it looks like you are using Python 3 (which is great). Can you confirm ? (The behavior for division is different which leads to different behaviors in your case) \$\endgroup\$ – Josay Mar 6 at 17:52
  • 1
    \$\begingroup\$ Thanks for the warm welcome:). Yes, I am using Python 3. \$\endgroup\$ – barciewicz Mar 6 at 17:57
11
\$\begingroup\$

In general, building a list using a loop of the form

some_list = []
for …:
    some_list.append(…)

… would be better written using a list comprehension.

Each interval always has two elements: a start time and an end time. These two-element lists would be better represented as tuples instead of lists. (Tuples have a connotation that they have a fixed length, whereas lists can grow to arbitrary lengths.)

Finally, I'd package the code into a function.

def intervals(parts, duration):
    part_duration = duration / parts
    return [(i * part_duration, (i + 1) * part_duration) for i in range(parts)]
\$\endgroup\$
7
\$\begingroup\$

Here are a few suggestions.

Write a function

Your code could be moved into a function on its own. It has the benefit of giving the code a clear name, a clear input, a clear output and we could go further and add documentation and tests.

def split_song(song_duration, num_of_parts):
    """Returns parts when a song of duration song_duration is split into num_of_parts parts."""
    part_duration = song_duration / num_of_parts
    parts = []
    marker = 0

    for _ in range(num_of_parts):
        part = [marker, marker + part_duration]
        marker += part_duration
        parts.append(part)
    return parts

assert split_song(20, 4) == [[0, 5.0], [5.0, 10.0], [10.0, 15.0], [15.0, 20.0]]
assert split_song(21, 4) == [[0, 5.25], [5.25, 10.5], [10.5, 15.75], [15.75, 21.0]]

Proper data structure

You are returning a list of list. In Python, there is a cultural difference in how tuple and list are used.

In our case, we know that each piece will contain 2 pieces of information: the begining and the end. It would be more relevant to use tuples here.

part = (marker, marker + part_duration)
\$\endgroup\$
6
\$\begingroup\$

Firstly you should be able to see that the left value in each part is the same as the right value in the previous part. This can be implemented by using the pairwise recipe:

def pairwise(iterable):
    "s -> (s0,s1), (s1,s2), (s2, s3), ..."
    a, b = tee(iterable)
    next(b, None)
    return zip(a, b)

From this you should be able to generate all the wanted numbers using a list, or generator, comprehension:

part_duration = song_duration / num_of_parts
parts = [i * part_duration for i in range(num_of_parts + 1)]

import itertools

def pairwise(iterable):
    "s -> (s0,s1), (s1,s2), (s2, s3), ..."
    a, b = itertools.tee(iterable)
    next(b, None)
    return zip(a, b)

def song_segments(duration, segments):
    delta = duration / segments
    return pairwise([i * delta for i in range(segments + 1)])


print(list(song_segments(20, 4)))
\$\endgroup\$
6
\$\begingroup\$

A few things:

First, you can make use of the third parameter of range, which is the step, IF you can guarantee that part_duration is an integer (which is the case for the example you posted here):

# Integer division
part_duration = song_duration // num_of_parts
parts = []

# I rearranged this a bit too
for i in range(0, song_duration, part_duration):
    part = [i, i + part_duration]
    parts.append(part)

print(parts)
# [[0, 5], [5, 10], [10, 15], [15, 20]] # Note they're integers

Note how this is just a transformation from a range to a list though. If you're transforming one collection to a list, list comprehensions should come to mind:

# List comprehension split over two lines
parts = [[i, i + part_duration]
         for i in range(0, song_duration, part_duration)]

print(parts)
# [[0, 5], [5, 10], [10, 15], [15, 20]]

If you can't guarantee integer steps though, I'm not sure of a good way. Unfortunately, Python doesn't allow fractional steps for its range.

\$\endgroup\$

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.