Bottom Page

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())))
Quote
#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.
Quote
#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.
Quote

Top Page

Possibly Related Threads...
Thread Author Replies Views Last Post
  Batch file not running python script in task scheduler davork 3 315 May-09-2019, 12:53 PM
Last Post: Gribouillis
  How to get coroutine from a task, and an object from a coroutine? AlekseyPython 2 212 Mar-23-2019, 01:41 PM
Last Post: AlekseyPython
  Hard time with prime factors in the task. blackknite 1 261 Jan-23-2019, 04:15 PM
Last Post: ichabod801
  Windows 10 Task Scheduler Error mypython 1 439 Aug-11-2018, 11:01 PM
Last Post: ichabod801
  simple code task dan123445 4 664 Jun-13-2018, 02:20 PM
Last Post: ljmetzger
  a debugging task Skaperen 0 509 Feb-18-2018, 08:06 AM
Last Post: Skaperen
  Script for automated task help darkitecture 2 1,051 Mar-26-2017, 07:02 PM
Last Post: micseydel
Question Drawing a sine curve in memory view of Task Manager Lukiewookie 5 3,352 Sep-20-2016, 08:47 AM
Last Post: Ofnuts

Forum Jump:


Users browsing this thread: 1 Guest(s)