Python Forum
Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Interview task
#1
Hello Folks,

Recently i got a python assessment task from a company. Though the code was right but unfortunately i was not selected for the next round because i did not follow the python standards.
I will post the code below please tell me how can i improve my code readability.

#********************** This is the main program************************************ 

import sys
import classModule as cm

classObject  = cm.assignmentTask()                  # object instantiation to the class assignmentTask()
fileName     = sys.argv[1]                          # reads file path passed as the arguments right after the find_subsequence.py
subSeqLength = int(sys.argv[2])                     # reads the maximum subsequence length passed as second argument
classMethod  = sys.argv[3]                          # reads the class method to further evaluate (values or differences)

f_id = open(fileName,"r")                           # file object for reading a input file 
lst1 = f_id.read()                                  # read content of the file till EOL
lst1 = [int(val) for val in lst1.split()]           # list type casting to 'int' type 

if(classMethod == "values"):                        # compare command line argument to invoke a class method
    classObject.values(lst1,subSeqLength)           # method invoked for computing highest sum of absolute values for given subsequence 
elif(classMethod == "differences"):
    classObject.differences(lst1,subSeqLength)      
else:
    raise Exception("Argument passing error: Improper classMethod") # raise exception if a unknown method is passed 
#********************** This is the module program************************************

class assignmentTask():
    
    def values(self, l1, n):         
        self.l1 = l1
        self.n = n
        d = dict()
        tmp_sum = []

        for i in range(4,n+1):                    
            for j in range(len(l1)-i+1):
                l2 = l1[j:j+i]
                tmp_sum.append(sum(l2))         
            d[i] = max(tmp_sum)        # maps maximum value for various subSequence(n) combinations     
            tmp_sum = []
            
        print("{}".format(max(d.values())))


    def differences(self, l1, n):
        self.l1 = l1
        self.n = n
        tmp_l2 = []
        d = dict()

        for i in range(len(l1)-(n-1)):
            l2 = l1[i:i+n]
            for j in range(len(l2)-1):
                tmp_l2.append(abs(l2[j] - l2[j+1]))   
            d[i] = sum(tmp_l2)       # maps the sum of absolute difference for various subSequence(n) combinations
            del tmp_l2[:]

        print("{}".format(max(d.values())))
Reply
#2
Sorry about the interview. You'll do better next time.

Supposing that they're referring to PEP 8 standards, here are the issues I see:

  1. Variable names should be lowercase with underscores to improve readability:
    class_object = cm.assignmentTask()
    file_name = sys.argv[1]
    sub_seq_length = int(sys.argv[2])
    class_method = sys.argv[3]
  2. Also with the above, do not use more than one space between an operator and the operands to align operators on multiple lines.
  3. Conditionals do not need parentheses.
    if class_method == "values":
        class_object.values(lst1, sub_seq_length)
    elif class_method == "differences":
        class_object.differences(lst1, sub_seq_length)    
  4. Class names should use CapWords format:
    class AssignmentTask():
  5. Lines should not exceed 79 characters. Many of the inline comments extend beyond that limit.
  6. Comments should be complete sentences with subject and verb and a capitalized first letter and ending punctuation.
  7. Inline comments should be used sparingly.

On a side note, I'm not understanding the purpose of assigning attributes in assignmentTask. Neither attribute gets used because all invocations are of the arguments passed in instead.

Also, more descriptive names than "lst1" and "l1" may have benefited you.
Reply
#3
In addition to stullis's comments/recommendations:
  • There are no doc-strings. for example at the moment we cannot fully understand what your class and methods are expected to do.
  • At the same time using inline comments in the main program for obvious things
  • Using single-letter variable/attribute names
  • not using with context manager when open files
  • using if/else, instead of try/except in the main program when calling respective method - better to ask for forgiveness than for permission.
  • related to previous bullet - raising general Exception, when the actual AttributeError would do
  • The way you do it it doesn't make sense to use class. If it was requirement I guess they expected to pass sequence l1 and n at instantiation. This way you would not assign self.l1 and self.n in each method, but only once in __init__().
  • Also, I think it's better that the two methods return value, not print (unless it was specifically requested)
  • magic numbers - e.g. where this 4 on line 11 comes from? Probably it will be clear if we know the actual assignment, but still it's not clear what you do
  • I am almost certain that your code can be improved even further if the full task is clear. e.g. why using dict when list would do? You don't make use of the dict

This is non-exhaustive list, just some random thoughts while looking at your code. Please, don't take offense.
If you can't explain it to a six year old, you don't understand it yourself, Albert Einstein
How to Ask Questions The Smart Way: link and another link
Create MCV example
Debug small programs

Reply


Possibly Related Threads…
Thread Author Replies Views Last Post
  count certain task in task manager[solved] kucingkembar 2 1,083 Aug-29-2022, 05:57 PM
Last Post: kucingkembar
  Schedule a task and render/ use the result of the task in any given time klllmmm 2 2,033 May-04-2021, 10:17 AM
Last Post: klllmmm
  How to create a task/import a task(task scheduler) using python Tyrel 7 3,626 Feb-11-2021, 11:45 AM
Last Post: Tyrel

Forum Jump:

User Panel Messages

Announcements
Announcement #1 8/1/2020
Announcement #2 8/2/2020
Announcement #3 8/6/2020