Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Let's take that step by step:

def join(l, sep):
 out_str = ''
 for i, el in enumerate(l):

Here, why do you need the enumerate? You could write for el in l:

 out_str += '{}{}'.format(el, sep)

.format is not super efficient, there are other methods. You can have a look at This question This question for some researches and benchmarks on performances.

 return out_str[:-len(sep)]

This makes little sense for l = [] if len(sep) > 1. ''[:-1] is valid, and returns '', because python is nice, but it is not a very good way of getting around that limit case.

In general, adding something just to remove it at the end is not great.

Creating an iter, looking at the first value, then adding the rest, as it has been suggested in other answers, is much better.

I would also recommend writing some unit tests, so that you can then play around with the implementation, and stay confident that what you write still works.

Typically, you could write:

# Empty list
join([], '') == ''
# Only one element, -> no separator in output
join(['a'], '-') == 'a'
# Empty separator
join(['a', 'b'], '') == 'ab'
# "Normal" case
join(['a', 'b'], '--') == 'a--b'
# ints
join([1, 2], 0) == '102'

Let's take that step by step:

def join(l, sep):
 out_str = ''
 for i, el in enumerate(l):

Here, why do you need the enumerate? You could write for el in l:

 out_str += '{}{}'.format(el, sep)

.format is not super efficient, there are other methods. You can have a look at This question for some researches and benchmarks on performances.

 return out_str[:-len(sep)]

This makes little sense for l = [] if len(sep) > 1. ''[:-1] is valid, and returns '', because python is nice, but it is not a very good way of getting around that limit case.

In general, adding something just to remove it at the end is not great.

Creating an iter, looking at the first value, then adding the rest, as it has been suggested in other answers, is much better.

I would also recommend writing some unit tests, so that you can then play around with the implementation, and stay confident that what you write still works.

Typically, you could write:

# Empty list
join([], '') == ''
# Only one element, -> no separator in output
join(['a'], '-') == 'a'
# Empty separator
join(['a', 'b'], '') == 'ab'
# "Normal" case
join(['a', 'b'], '--') == 'a--b'
# ints
join([1, 2], 0) == '102'

Let's take that step by step:

def join(l, sep):
 out_str = ''
 for i, el in enumerate(l):

Here, why do you need the enumerate? You could write for el in l:

 out_str += '{}{}'.format(el, sep)

.format is not super efficient, there are other methods. You can have a look at This question for some researches and benchmarks on performances.

 return out_str[:-len(sep)]

This makes little sense for l = [] if len(sep) > 1. ''[:-1] is valid, and returns '', because python is nice, but it is not a very good way of getting around that limit case.

In general, adding something just to remove it at the end is not great.

Creating an iter, looking at the first value, then adding the rest, as it has been suggested in other answers, is much better.

I would also recommend writing some unit tests, so that you can then play around with the implementation, and stay confident that what you write still works.

Typically, you could write:

# Empty list
join([], '') == ''
# Only one element, -> no separator in output
join(['a'], '-') == 'a'
# Empty separator
join(['a', 'b'], '') == 'ab'
# "Normal" case
join(['a', 'b'], '--') == 'a--b'
# ints
join([1, 2], 0) == '102'
Source Link
njzk2
  • 721
  • 3
  • 11

Let's take that step by step:

def join(l, sep):
 out_str = ''
 for i, el in enumerate(l):

Here, why do you need the enumerate? You could write for el in l:

 out_str += '{}{}'.format(el, sep)

.format is not super efficient, there are other methods. You can have a look at This question for some researches and benchmarks on performances.

 return out_str[:-len(sep)]

This makes little sense for l = [] if len(sep) > 1. ''[:-1] is valid, and returns '', because python is nice, but it is not a very good way of getting around that limit case.

In general, adding something just to remove it at the end is not great.

Creating an iter, looking at the first value, then adding the rest, as it has been suggested in other answers, is much better.

I would also recommend writing some unit tests, so that you can then play around with the implementation, and stay confident that what you write still works.

Typically, you could write:

# Empty list
join([], '') == ''
# Only one element, -> no separator in output
join(['a'], '-') == 'a'
# Empty separator
join(['a', 'b'], '') == 'ab'
# "Normal" case
join(['a', 'b'], '--') == 'a--b'
# ints
join([1, 2], 0) == '102'
lang-py

AltStyle によって変換されたページ (->オリジナル) /