Python backend interview: task management system for workers

I've recently applied to a backend position and I was asked to complete a take-home interview question. I thought the question was rather simple and I completed in about an hour although I was told it could take up to 4 hours.

I got a response back recently stating that my answer does not demonstrate the necessary skill-level required. Looking back at my answer, I am not exactly sure where I should improve.

Please take a look a look and see what you would have done differently.

The orignal question:


Description:


A supervisor is using the TaskManagementSystem to manage tasks for his
workers. The system can add users, assign tasks to users, and get
task(s) that belong to a specific user.


Restriction:


You can not modify user or task class. self.users and self.tasks in
TaskManagementSystem has to remain as lists. You can introduce new
methods and class variables. Goals:



-
Fix the syntax and semantic bugs that prevent the program from running correctly.

-
Change get_user_tasks() to return list of task names instead of task objects.

-
Add error handling to add_user(), add_task(), and get_user_tasks() methods

-
Optimize TaskManagementSystem to handle large number of users and tasks


Code provided at interview:

```
#!/usr/bin/env python2.7
# encoding: utf-8

class user(object):

def __init__(self, user_id, name):
'''
Task object. This code can not be modified.
@ user_id: int
@ name: string
'''
self.user_id = user_id
self.name = name

class task(object):

def __init__(self, user_id, task_name):
'''
Task object. This code can not be modified.
@ user_id: int
@ task_name: string
'''
self.user_id = user_id
self.task_name = task_name

class TaskManagementSystem(object):

def __init__(self):
self.users = [] # stores a list of user objects
self.

Let’s tackle the four points in turn:

-
Syntax and semantic bugs. I assume this was the indentation problems, which you seem to have fixed correctly.

-
Change get_user_tasks() to return a list. This is pretty good.

But you return None if the user isn’t found – that’s not a list, and could be interpreted as meaning that the user exists, and doesn’t have any tasks assigned, which is wrong. You should throw a UserNotFound exception (see below).

And this is a good use of list comprehensions, but will be slow if you have lots of tasks (again, see below).

-
Add error handling. Your approach to error handling is wrong – rather than printing messages to stdout (which are very hard for calling programs to hook into, viz. try … except), you should throw suitable exceptions.

For example: your add_user method could throw a DuplicateUser exception if I try to add a user who already exists. This is much more useful for control flow.

And your error messages could be more specific – although it’s obvious which user I’m trying to re–add in a small script, it that shows up in a massive log file, it would be nice to know which user was added twice. That’s useful for debugging.

-
Optimise the class for lots of tasks and users. This seems to be the key task, and it doesn’t seem like you’ve done anything in this area. I’d have swapped out the two attributes for more appropriate data structures (probably dicts), but apparently that’s against the rules.

You could have protected attributes _users and _tasks that contain your proper data structures, and add properties users and tasks that return the correct lists. That way the public interface is preserved, but you have a more efficient implementation.

You’d need to use the @property and @foo.setter decorator for that – not completely trivial, but not impossible either.

I wasn’t on this interview panel, but the key mistakes seem to be the error handling (use exceptions, not printing) and the lack of meaningful changes to improve scalability.

And a few other small things I spotted:

-
Read PEP 8, the Python style guide. The names of the user and task classes should be CamelCase.

-
The docstring of the user object is incorrect. Was that a deliberate mistake you were meant to catch?

-
Your add_user() method assigns a user ID based on the existing user IDs. This might be a problem if you later add a delete_user() method – for example:

create user 'alice' (id 1)
create user 'bob' (id 2)
delete user 'bob' (id 2)
create user 'carol' (id 2)

At different points in time, I had two users with the same ID – that might cause confusion. It would be nice if IDs were reasonably unique. Maybe use hash(name)?

-
This isn’t wrong, but it’s a bit unusual:

except(IndexError):

except is a statement, not a function.

-
I know you’re not allowed to modify the user or task objects, so this is an aside: those two objects could be replaced by namedtuples with multiple benefits:

• Reduced occupancy

• Nicer repr()’s

• Equality/comparison semantics

StackExchange Code Review Q#122849, answer score: 15