0

Recently, I've got a code test from ABC. Please take a look at this one and give me your idea.

Find the max value of an array of N integers.

E.g. [ 1, -6, 2, 0, 1011, -355]

Require: the max value is a multiple of 3 and the code focuses on the correctness not performance.

def solution(A):
    try:
        if not isinstance(A, list):
            raise TypeError
        max_value = max([x for x in A if x % 3 ==0])
        return max_value
    except Exception as e:
         print(f"{A} is not a list")
         print(e)

So does my code fulfill the requirements or Did I miss any edge case? P/s: They announced that was fail and only got 22/100 pts.

Scorpisces
  • 69
  • 1
  • 9
  • 1
    I'm not sure what you're asking for – Sayse Apr 21 '22 at 07:41
  • So the question is basically to give the largest number that fullfills the requirements? – Jhanzaib Humayun Apr 21 '22 at 07:43
  • @JhanzaibHumayun: yes. Please review my code, Does it fulfills the requirements? – Scorpisces Apr 21 '22 at 07:45
  • I have another interpretation, but it will depend on the original statements. You find the max value of the N int array as max(a) then you require that max(a) % 3 == 0 if so you return it, if not return None. under this twisted interpretation, it doesn't fulfil the requirements. – Ziur Olpa Apr 21 '22 at 08:02
  • 1
    You have to ask the people who graded you. If performance and code style are not an issue, then I don't see a reason to fail this. – 9769953 Apr 21 '22 at 08:04
  • I think of two issues that might come up; 1. does 0 is a multiple of 3? and 2. It doesn't seem that your code is checking if the list contains only integers. – Ze'ev Ben-Tsvi Apr 21 '22 at 08:07

3 Answers3

0

Honestly I can't see anything wrong with your max code, my guess could be that they are not passing in a list object in their tests - so your check is failing. Instead of testing for list, you could test for iterable.

See:

  1. In Python, how do I determine if an object is iterable?
  2. https://docs.python.org/3/library/functions.html#max
def solution(A):
    try:
        # ensures A is iterable
        _ = iter(A)
        return max([x for x in A if x % 3 ==0])
    except TypeError as e:
        print(f"{A} is not an iterable")
        print(e)

They could, for example, pass a tuple of (-5, 1, 24, 6), which would fail in your implementation.

Polymer
  • 1,108
  • 1
  • 9
  • 17
  • I don't see the point of your `_ = iter(A)`. Can you show an example where it makes a difference? – Kelly Bundy Apr 22 '22 at 04:17
  • @KellyBundy it was included to act as a better type check than the one given in the question. Eg running the method as such `solution("123")` would result in an error as `"123"` is not iterable. As we don't need the result, I used a throwaway `_` variable. It's an improvement over their `instanceof` implementation as it allows for any iterable, not just lists. – Polymer Apr 26 '22 at 01:06
  • But your list comprehension includes that "check" anyway. Again: Can you show an example where it makes a difference? – Kelly Bundy Apr 26 '22 at 01:39
0

I think the Exception is bad formulated, it prints "A is not a list" but just put a list like:

a = [ 1, -6, "hi", 0, 1011, -355]

This function will print:

[ 1, -6, "hi", 0, 1011, -355] is not a list

When it is a perfectly valid list.

If you assume that they will only provide a "array(list) of N integers" then the exception pattern makes no sense in the first place, if you assume that you can have bad inputs, then you need to handle a case as the one described.

pd: as a comment pointed out:

["1", "3", "6", "-20"]

Is a list of "integers" (type strings) that will also fail.

Second error:

If they really meant that they will provide an array/np.array then it will also fail in the isinstance as an array is not a list, as others pointed out.

Ziur Olpa
  • 1,839
  • 1
  • 12
  • 27
0

The question is flawed.

Pure Python doesn't have arrays - it has lists (unless the inquisitor is referring to the array module - which I doubt). However, let's assume that they are synonymous in this context.

Also, the question doesn't tell you what the behaviour should be if there is no answer - i.e., what if none of the numbers in the list are multiples of 3?

Why would you test the input parameter for its type? The question tells you what type it will be.

So I would suggest that it's really simple:

def solution(A):
    return max(x for x in A if x % 3 == 0)

Note that max is passed a generator rather than a list which although not important in this case, is potentially more efficient than building a list

DarkKnight
  • 19,739
  • 3
  • 6
  • 22
  • In a cross-platform coding test. They will give a common question in order we can resolve it in many languages, so that can be understood. – Scorpisces Apr 21 '22 at 08:16
  • Pure python does have arrays built in, just do "import array" , nothing to install needed. – Ziur Olpa Apr 21 '22 at 08:17
  • @PabloRuiz Maybe that's the issue. Perhaps the argument is an *array* type either from the built-in *array* module or from *numpy*. Who knows? It's a bad question regardless – DarkKnight Apr 21 '22 at 08:18
  • @LancelotduLac, I agree they give a python built in array and then you are checking if it is a list... so that will fail. – Ziur Olpa Apr 21 '22 at 08:26
  • @PabloRuiz Note that I have deliberately omitted the *isinstance* check on the grounds that if the parameter passed is either an array or list then the code will function correctly. I'll edit my initial comment in the answer – DarkKnight Apr 21 '22 at 08:33