1
\$\begingroup\$

Problem statement: Given an integer list from 0 to 100 find the missing Element.

I saw this problem on reddit on a compilation of interview questions, tried it out for fun. Mainly tried to generalize the code for any boundry(not just 100) and multiple missing arguments. Focused on readability and writing docstrings based on Google's recommendations. Did I hit my targets? How can and should my code be improved. Thanks!

"""
    Problem: Given a list from 0 to 100, find the missing element
"""

def makeIntArray(excludedValues, boundry):
    return [i for i in range(boundry + 1) if i not in excludedValues]

def compareToComplete(partial):
    """ Compares a possibly partial list to its complete version

        Args:
            partial: List containing integer type, all duplicates will be ignored

        Returns: 
            Set of unique  missing elements or empty set if partial is complete  

        example:
             partial = [1,2,3,4,6,8,10], complete = [0,1,2,3,4,5,6,7,8,9,10]
             returns: {0,5,7,9}
    """
    partial.sort()
    return {i for i in range(partial[-1] + 1)}.difference(set(partial))
\$\endgroup\$
3
\$\begingroup\$

don't change the input unless explicitly stated

partial.sort() works in-place. This might cause some nasty surprises for someone who uses this function and did not expect this.

The reason you sort this is also only to take the last element. max(partial) would suffice here. If partial is sorted, you can work with a different algorithm using the sortedness

set comprehension

a set comprehension is nice, but here a set(range(partial[-1] + 1)) would suffice

set.difference

there is no need to cast partial to a set

so the complete method reduces to

return set(range(max(partial) + 1)).difference(partial)

alternative method

If partial is sorted, you can work with 2 iterators

from itertools import count

def compare_to_complete_generator(partial):
    complete_iterator = count()
    partial_iterator = iter(partial)

    element_complete= next(complete_iterator)
    element_partial = next(partial_iterator)

    while True:
        while element_partial != element_complete:
            yield element_complete
            element_complete = next(complete_iterator)
        element_complete = next(complete_iterator)
        try:
            element_partial = next(partial_iterator)
        except StopIteration:
            return

list(compare_to_complete(partial))
[0, 5, 7, 9]

For large lists, this has the advantage that you don't have to look for the max and construct a set with max elements. This also accepts any sorted iterable or iterator, not just lists. if the input is not sorted, you can use sorted, but then you lose the advantages of this method.

\$\endgroup\$
0
\$\begingroup\$

Well, firstly your code doesn't "run" as you have no entry point (ala if __name__...), secondly, you have no test code to even prove your code works as you believe it to, and thirdly - you have a function makeIntArray which is not used in your code.

Regarding your points - readability - your function names need improving. compareToComplete means what? Compare to...? What is Complete? You're passing in a partial variable but how do you validate against the complete variable? It's not in the function signature.
Also, you're using CamelCase and not snake_case. Check out the PEP8 conventions and adhere to them as best you can when writing Python.

The docstring of compareToComplete explains the function very well, but if a programmer has to read the docstring to understand your code, then your code has failed to tell the story of what it does. Write your code without a docstring, and add the docstring when you need to explain WHY you have it.

Docstrings that say "I print out a variable" and the code is named print_variable(variable) make the docstring redundant and should be deleted. As a famous programmer said "comments lie, but the code tells the truth".

The other point - improving your code - I echo what is mentioned above, write your code to explain what it does, naming your variables appropriately.

Other than that, keep writing code. The more you write, and the more you read, the better you get. Also, hang out on code review and try your hand at giving answers too, read what others say on other questions to learn and improve.

Good luck!

\$\endgroup\$
3
  • \$\begingroup\$ So I was following the Google format listed here on SO stackoverflow.com/a/24385103/9333612, why is it best not to use this format over the Pep8 one, mine makes everything pretty clear I though. Also the name comparToComplete is in itself not the best name however if the function is well documented on it's purpose I think the name becomes more clear after reviewing the docstring. The same argument "compareToComplete what" could easily be used in the itertools module or any module for that. For example "chain what or accumulate what?" You need to read the documentation to describe it. \$\endgroup\$ – Anonymous3.1415 Oct 5 '18 at 9:37
  • \$\begingroup\$ @Anonymous3.1415 Using an alternative coding standard is fine. PEP8 is just one option, you should just be consistent to whichever standard you use. \$\endgroup\$ – Jack Aidley Oct 5 '18 at 10:32
  • \$\begingroup\$ and there is a difference in the format of the docstring, and the format of the function names, variables, etc. you can follow pep-8 for the code, and the google documentation style for the docstring \$\endgroup\$ – Maarten Fabré Oct 5 '18 at 10:35

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.