# Finding students who appear on a list

I'm new in Python and consequently I am searching for a specific solution for code development. I already asked some pythonic question today and want to present my whole problem. I have found some solution, but I really have interest to develop easier and clearer code in Python, so all improvements are welcome.

For example, in some school, the students are ordered to some of 3 different classes. Now I have some set of graduated students and want to know, how many graduated students are in some class.

I create next code:

physic = ['id_1', 'id_2', 'id_4']
chemistry = ['id_5', 'id_7']
math = ['id_3', 'id_8', 'id_9']

classes = [physic, chemistry, math]


Every class has the different number of students (given by students IDs) and gratuated Classes are at the first empty. I combine it also to list of lists.

Then I use my code:

graduatedlist = ['id_2', 'id_5', 'id_7']

for pos2 in range(len(classes[pos1])):
if stud.find(classes[pos1][pos2]) != -1:


This code works, but does not look great... I would be very grateful for comments! For example, I tried to implement the idea with itertools.product():

for item_1, item_2 in itertools.product(range(len(graduated)), range(len(classes[item_1])):


I have a problem with a declaration of item_1.

I have also a feeling that the lists isn't the best solution for implementation, but for DataFrame I need the column of similar length and every class can have the different number of the students.

For the special interest are two points:

1. How I really can improve my for-loop?
2. What I can use instead of list declaration?
-
Hi, welcome to CodeReview.SE! Can you please provide your actual code and what it is supposed to do ? (At the moment, I have doubts that your code is working at all : there is a typo in gratuated and the variable item is not defined anywhere). – Josay Jul 28 '14 at 14:46
sorry, item is stud, I have to replace it... – Guforu Jul 28 '14 at 14:52

Re-organising the logic

As a rule of thumb, there is almost no need for range(len(x)) in Python. Thus :

for stud in graduatedlist:
for pos2 in range(len(classes[pos1])):
if stud.find(classes[pos1][pos2]) != -1:


definitly looks wrong and can easily be improved :

for stud in graduatedlist:
for stud2 in classes[pos1]:
if stud.find(stud2) != -1:


Now, instead of iterating over graduatedlist, it might make more sense to iterate over the classes we are considering and then, for each class, consider the student and for each student, check if he has graduated.

for pos1, grad_list in enumerate(graduated):
for stud in classes[pos1]:


Even better, you don't need to prepopulate graduated = [graduatedPhysic, graduatedChemistry, graduatedMath] : it makes things akward as you have to deal with indices. You could just add elements to the list as you go :

graduated = []

for c in classes:
for stud in c:


Making things more beautiful

Now, in Python, this looks typically like something can be written in a more concise way using list comprehension :

graduated = []

for c in classes:


Or in a more concise way :

graduated = []
for c in classes:


And here we have the same pattern again :

graduated = [[stud for stud in c if stud in graduatedlist] for c in classes]


Using the right tool/data structure

Now, things can probably be done in a much smarter way using the right data structure. You want to quickly change if some element belongs or not to something. sets is what you should be using and you want to compute intersections.

physic = {'id_1', 'id_2', 'id_4'}
chemistry = {'id_5', 'id_7'}
math = {'id_3', 'id_8', 'id_9'}

classes = [physic, chemistry, math]


Or more succintly :

graduated = [c & graduatedlist for c in classes]

-
Thank you very much, this is just great! – Guforu Jul 28 '14 at 15:24
Nice. I like this one better than my alternative. Thanks for sharing. – dss539 Jul 28 '14 at 15:34

I would strongly recommend creating a class.

class Student:
self.id = id


graduatedchemistry = [student for student in chemistry if student.graduated]


To cleanly get graduates from all classes then you can make a function

def getgraduates(students):
return [student for student in students if student.graduated]


Respect PEP-8, fields should be lowercase. So, student.graduated, not student.Graduated. – Stefano Sanfilippo Jul 28 '14 at 15:03
It's just a convention: classes are CapitalizedCamelCase, modules, variables and functions are lower_snake_case, unless they are module level variables, which means UPPER_SNAKE_CASE. There is no specific reason it was choosen and it's not the best, simply is what most folks around have agreed to use. By adhering to it, you make it simpler for others to identify the elements of your code. – Stefano Sanfilippo Jul 28 '14 at 15:16
@dss539 - I think it's actually somewhat redundant to do that, since in Python, you always prefix fields and methods with self anyways. As a result, you can always immediately identify whether or not something is a field or method regardless of what naming scheme you pick. In any case, style guidelines are always going to be somewhat arbitrary -- the real benefit of only occurs when everybody actually uses them. IMO the specific guidelines don't matter as much, but the consistency does. – Michael0x2a Jul 28 '14 at 17:25