Script for finding cheating students in a quiz

Below is a script to find cheating students in a quiz in a daily moodle activity log exported in .xls. It works fine and is written in a procedural way.

Essentially the script isolates the activity log entry in a worksheet for students listed in another worksheet, identifies when they are taking a quiz (looking ahead in the activity log) and if they are taking an unusually long time or doing something different while taking a quiz (they will complete). These different scenarios are then highlighted in a different row colour in an xl file listing all activity for each student in the list. The script also writes to a .txt file, logging a summary for each student and all quizzes completed and the students listed who did not take any quizzes.

The questions are simple:

• How can I make it run faster?

• How can I write it in a OOP paradigm, and will it run faster?

```
#!/usr/bin/env python
# -- coding: utf-8 --

# The master data set is contained in excel file ('moodle.xls'). The toponomy for the activity is the one in a 2.5+ moodle instance. The colums in excel follow the format
# columns = module name/date,time/(Not used):e.g. IP address/name student/activity type (full)/activity name
# rows = chronological order in time period

# The class (moodlecheat) returns: a log containing a summary of cheats per students, a set of individual xl files with a log per student with quiz question highlighted.
# Each row is colour coded to facilitate detection: yellow/any valid quiz question answered | light yellow/any question which is answered in more than 2 minutes | grey/any non quiz activity during a quiz | Red/any activity taking longer than 5 minutes during a quiz
# The data listed in a log (log.tx) which details the running times per module and the activity for all students highlighting any suspected cheating

# Worksheet 1 = xl report for moodle activity log for the time period e.g. a day
# Worksheet 2 = Name of users/students whose quiz data is to be mined out

#

As the other answer mentioned, PEP8 will help your readability. Because you already have been informed about PEP8, I (hopefully) won't spend too long on it. Instead I'll just list some points to really look for in that document:

• Naming conventions: PascalCase for classes and underscores_in_names for basically everything else.

• Descriptive variable names. Even if they are temporary variables, give them relatively descriptive names.

• Space is your friend: a single space between methods and to split up logical sections of code. However, take care not to overdo it.

• Try to stick to 80 characters per line. This is probably the least necessary of the style suggestions. It really is only matters for people who have really small screens.

There are more style suggestions I can make, but as you indicated in a comment above, you plan on reading and following PEP8, so I won't inundate you with its conventions right away.

Onto, improvements.

-
ALWAYS USE with when dealing with files. You currently use open. This technically is fine, however it much more prone to bugs. You are actually guilty of one of the bugs: you open a file pointer, but do not call close on it:

def logmessage(self,message):
    # writes  in log.txt at logfilepath
    self.message=message
    flog=open (self.logfilepath+'log.txt', "a")
    flog.write('\n'+self.message)

Using with your logmessage function (PEP8-ed) looks as such:

def log_message(self, message):
    self.message = message
    with open(os.path.join(self.log_filepath, 'log.txt'), 'a') as file:
        file.write('\n' + self.message)

-
In the point above, notice how I used os.path.join. Use this function when you are creating filepaths as it automatically uses the OS-specific separator when joining the arguments.

-
Use docstrings. You have quite a few comments that describe your methods and what they do. This is nice. However, if you use docstrings, you get some added functionality:

>>>def foo():
...    '''This is a sample docstring.'''
...    print('Hello World!')
>>>help(foo)
Help on function foo in module __main__:

foo()
    This is a sample docstring

-
You do a lot of string concatenation. While the performance of concatenation against string formatting is debatable, the Pythonic way to embed variable information into string is to use the str.format function. Example:

# Here is how you do this.
self.logmessage (m[3]+' retook the quiz '+m[5]+  ' at '+m[1].split(', ')[1]+' - This is an issue if the quiz is not a practice quiz')

# Here is how `format` does this.
self.logmessage('{} retook the quiz {} at {} - This is an issue if the quiz is not a practice quiz'.format(m[3], m[5], m[1]))

Also, when printing out lists and the delimiter stays the same between each element, use the join function:

# Here is how you do this.
d4=d2.split(' ')[0]+' '+d3+' '+d2.split(' ')[2]+' '+d2.split(' ')[3]+d2.split(' ')[4]

# Here is how `join` does this.
d4 = ' '.join(d2.split())

-
Variables are nice. You do a lot of this:

foo.split()[0] + ' ' + foo.split()[1] + ' ' ...

This can get quite intensive because you, theoretically, are splitting your string n times, where n is the number of spaces in your string. Instead of splitting the string each time you are accessing a new index, simply store the list once:

split_foo = foo.split()

As for OOP suggestions, take a look at your code and anywhere you use specific indices in a list to represent a constant form of data, create (or merge it into) a class.

Example: In your activestudents list you hold a list of two values: the student's name and a flag that says if they are doing an activity. This can be combined with your inactivestudents list into a single class:

class Student(object):
    def __init__(self, name, active=True, taking_quiz=False):
        self._name = name
        self._active = active
        self._taking_quiz = taking_quiz

    def is_active(self):
        return self._active

    def is_taking_quiz(self):
        return self._taking_quiz

Now you can have a single list of students and to work on only the active ones simply do:

active_students = [student for student in self.students if student.is_active()]

You could also create a Quiz class and have the Student class hold a list of Quizes with the number of questions they have completed.

StackExchange Code Review Q#53844, answer score: 6