Skip to main content
added 362 characters in body
Source Link
toolic
  • 16.4k
  • 6
  • 29
  • 221

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):

Similarly for the calls to max:

            if max(N[i+1:j], default=0) > N[i]:
                valid -= 1
            elif max(N[i+1:j], default=0) > N[j]:

This is simpler:

            max_num = max(N[i+1:j], default=0)
            if  max_num > N[i]:
                valid -= 1
            elif max_num > N[j]:

Input checking

If the input to your function is known to be good, in other words, if you validate the input before passing it to the function, then ignore the following. Otherwise, consider checking the input in the function. For example, if the input contains a negative value, the output is not what I would expect. Obviously, there is no such thing as a negative height, but you might want to report it to the user. Checking would take some time to do, but it would only be done once for each item in the list.

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):

Input checking

If the input to your function is known to be good, in other words, if you validate the input before passing it to the function, then ignore the following. Otherwise, consider checking the input in the function. For example, if the input contains a negative value, the output is not what I would expect. Obviously, there is no such thing as a negative height, but you might want to report it to the user. Checking would take some time to do, but it would only be done once for each item in the list.

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):

Similarly for the calls to max:

            if max(N[i+1:j], default=0) > N[i]:
                valid -= 1
            elif max(N[i+1:j], default=0) > N[j]:

This is simpler:

            max_num = max(N[i+1:j], default=0)
            if  max_num > N[i]:
                valid -= 1
            elif max_num > N[j]:

Input checking

If the input to your function is known to be good, in other words, if you validate the input before passing it to the function, then ignore the following. Otherwise, consider checking the input in the function. For example, if the input contains a negative value, the output is not what I would expect. Obviously, there is no such thing as a negative height, but you might want to report it to the user. Checking would take some time to do, but it would only be done once for each item in the list.

added 528 characters in body
Source Link
toolic
  • 16.4k
  • 6
  • 29
  • 221

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):

Input checking

If the input to your function is known to be good, in other words, if you validate the input before passing it to the function, then ignore the following. Otherwise, consider checking the input in the function. For example, if the input contains a negative value, the output is not what I would expect. Obviously, there is no such thing as a negative height, but you might want to report it to the user. Checking would take some time to do, but it would only be done once for each item in the list.

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):

Input checking

If the input to your function is known to be good, in other words, if you validate the input before passing it to the function, then ignore the following. Otherwise, consider checking the input in the function. For example, if the input contains a negative value, the output is not what I would expect. Obviously, there is no such thing as a negative height, but you might want to report it to the user. Checking would take some time to do, but it would only be done once for each item in the list.

added 473 characters in body
Source Link
toolic
  • 16.4k
  • 6
  • 29
  • 221

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

The code layout is good.

It is clear from the code that the function accepts a list of integers and returns a list of integers.

However, you should add a docstring describing more details about the returned list. You could add details for the following:

  • It returns a list of the same length as the input list (if that is the case)
  • It appears to return a valid indication for each item in the input list. I think of "valid" as being a boolean (1 or 0). What does 2 mean? What does 5 mean?

Essentially, it would be better to more precisely define what you mean by "valid".

DRY

Since your function does not modify the input list N, its length remains the same. However, your code calls len(N) multiple times:

for i in range(len(N)):
    valid = 0
    for j in range(i+1, len(N)):

Instead, you can set the length to a variable at the top of the function:

nums = len(N)
for i in range(nums):
    valid = 0
    for j in range(i+1, nums):
added 27 characters in body
Source Link
toolic
  • 16.4k
  • 6
  • 29
  • 221
Loading
Source Link
toolic
  • 16.4k
  • 6
  • 29
  • 221
Loading