2

Please help me see what I am missing below. I first created three objects of my class and added them to a collection list. Before creating any additional objects, I want to check to make sure that that person does not already exist in the list. If the person already exists, the person shouldn't be created again. I was hoping to achieve this check by doing if prompt_fname == person.fname and prompt_lname == person.lname:. Apparently, I'm not doing it correctly because the program still ran through and created the same person who already exists in the list. And it created this person two times. How can I modify to catch this so that a person already existing in the list is not created again. Also any new person should not be created again and again in each iteration of the loop. I'm new to programming so please don't leave out much detail in your answer. Thanks a lot.

class Person(object):

    personslist = []
    '''Creates a person object'''
    def __init__(self, firstname, lastname):
        self.lname = lastname.title()
        self.fname = firstname.title()
        Person.personslist.append(self)

    def __str__(self):
        return "{0} {1}".format(self.fname, self.lname)

    def __repr__(self):
        return "{0} {1}".format(self.fname, self.lname)


Person("Adamu", "Emeka")
Person("Femi", "Ojukwu")
Person("Wole", "Jonathan")


prompt_fname = "Adamu"
prompt_lname = "Emeka"

print(Person.personslist)

for person in Person.personslist:
    if prompt_fname == person.fname and prompt_lname == person.lname:
        pass
    else:
        Person(prompt_fname, prompt_lname)

print(Person.personslist)

Yields

[Adamu Emeka, Femi Ojukwu, Wole Jonathan]
[Adamu Emeka, Femi Ojukwu, Wole Jonathan, Adamu Emeka, Adamu Emeka]

Using Python 3.4.1

sedeh
  • 7,083
  • 6
  • 48
  • 65

5 Answers5

7

Your loop is checking each entry, and if that particular entry is not equal, it creates another instance. But if that particular entry is equal, it doesn't stop, it just continues to the next item, which won't be equal. That's why you actually got two extra entries at the end.

You can fix this by keeping a flag:

found = False
for person in Person.personslist:
    if prompt_fname == person.fname and prompt_lname == person.lname:
        found = True
        break
if not found:
    Person(prompt_fname, prompt_lname)

However, there is a far better way to do this: your way is very inefficient, as it requires a linear scan each time. Instead, keep a dictionary of objects keyed by their full name:

class Person(object):

    persons_dict = {}
    '''Creates a person object'''
    def __init__(self, firstname, lastname):
        self.lname = lastname.title()
        self.fname = firstname.title()
        fullname = "%s %s" % (self.fname, self.lname)
        Person.persons_dict[fullname] = self

and now you can simply check in one go:

if "%s %s" % (prompt_fname, prompt_lname) not in Person.persons_dict:
Daniel Roseman
  • 588,541
  • 66
  • 880
  • 895
  • Thanks Daniel for pointing out! I really should have seen why the list kept growing. I missed this one. – sedeh Aug 21 '14 at 14:47
  • Please see my implementation here: http://ideone.com/Vq5MpH. Am I implementing your suggestion correctly because it gives me: `{'Adamu Emeka': Adamu Emeka, 'Femi Ojukwu': Femi Ojukwu, 'Wole Jonathan': Wole Jonathan} {'Adamu Emeka': Adamu Emeka, 'Femi Ojukwu': Femi Ojukwu, 'Wole Jonathan': Wole Jonathan}`. – sedeh Aug 21 '14 at 14:57
  • But isn't that the output you want? The dict is the same before and after, meaning that you did not add a duplicate. – Daniel Roseman Aug 21 '14 at 15:01
  • I see now, the key:value pair is same. – sedeh Aug 21 '14 at 15:04
  • Do you know how I can get an ordered dict out of persons_dict? Dictionary is inherently unordered and the post [here](http://stackoverflow.com/questions/15733558/python-ordereddict-not-keeping-element-order?rq=1) suggests starting with a tuple. If I start with a tuple, I can't add new person object to it since tuple is [immutable](http://stackoverflow.com/questions/1538663/why-are-python-strings-and-tuples-are-made-immutable). Any further suggestions? Thanks. – sedeh Aug 21 '14 at 16:31
  • @DanielRoseman: I find this a very neat approach. However, I don't understand the value `self` in `Person.persons_dict[fullname] = self`. Wouldn't something like `1` or `TRUE` (or a counter!) be good enough (what matters is the index/key, right?) and consume less memory? Thanks. – msoutopico Nov 02 '19 at 00:57
1

This code doesn't do what it should:

for person in Person.personslist:
    if prompt_fname == person.fname and prompt_lname == person.lname:
        pass
    else:
        Person(prompt_fname, prompt_lname)

With for person in Person.personslist: it's going over the three Person() objects you've already created. The first one is 'Adamu Emeka' and so the names are equal, and the 'if' statement reaches 'pass'. However, the next item in personslist has the name 'Femi Ojukwu', and the names in the if are not equal, and so it reaches the else clause and creates a new object. The same happens for the third name. This is why you have two extra copies of Adamu Emeka.

See Daniel Roseman's answer for an alternate solution

Simon Fraser
  • 2,758
  • 18
  • 25
  • Note that the second code example fails if e.g., "Fred Jones" and "Mike Smith" are in the list already and the user attempts to add "Mike Jones" -- should succeed but doesn't with this design. – bgporter Aug 21 '14 at 15:05
  • Well spotted, I will remove that suggestion in favour of the one provided by Daniel Roseman – Simon Fraser Aug 21 '14 at 15:07
1

Check the full list each time before adding the name:

name = prompt_fname + prompt_lname

if not any(person.fname + person.lname == name for person in Person.personslist):
    Person(prompt_fname, prompt_lname)
Padraic Cunningham
  • 176,452
  • 29
  • 245
  • 321
0

Try with:

if prompt_fname.title() == person.fname and prompt_lname.title() == person.lname:

because you use the title() method in init of Person

Quentin THEURET
  • 1,222
  • 6
  • 12
  • Tried it. The result is same. A good catch though but since my input is properly capitalized, I think it didn't matter. – sedeh Aug 21 '14 at 14:32
-1

In your code once you find a person who is not Adamu Emeka, you add another Adamu Emeka to the list. That's why two Adamu Emeka's are added. Try the following:

alreadyExists = False;
for person in Person.personslist:
    if prompt_fname == person.fname and prompt_lname == person.lname:
        alreadyExists = True;
        break;
if not alreadyExists:
    Person(prompt_fname, prompt_lname)

A better way might be to use set so you don't have to perform a linear search. For set to work properly you have to implement __hash__ and __eq__ methods.

Anton Savin
  • 40,838
  • 8
  • 54
  • 90