3
\$\begingroup\$

this program seems to be working well so far, but I know it can be cleaner and more efficients with certain functions/methods. As I'm still beginning to code any feedback would be very welcome.

# Creating class named contacts with methods for editing and adding new 
# contacts. Also there are static methods for showing a specific or all
# contacts data and for showing if there's no contact saved on the address book.
class Contact:
 def __init__(self, name, surname, number, email):
 self.name = name
 self.surname = surname
 self.number = number
 self.email = email
 def edit_name(self, name):
 self.name = name
 return self.name
 def edit_surname(self, surname):
 self.surname = surname
 return self.surname
 def edit_number(self, number):
 self.number = number
 return self.number
 def edit_email(self, email):
 self.email = email
 return self.email
 @classmethod
 def add(cls, name, surname, number, email):
 return cls(name, surname, number, email)
 # This method prints prints all data of a specific entry it index is provided
 # otherwise it prints data of all entries saved in address book (fullnames, 
 # numbers and emails)
 @staticmethod
 def summary(index=0):
 if index == 0:
 for j in range(0, len(address_book)):
 print(address_book[j].name, address_book[j].surname, end=' / ')
 print(address_book[j].number, '/', address_book[j].email)
 else:
 print(address_book[index].name, address_book[index].surname, end=' / ')
 print(address_book[index].number, '/', address_book[index].email)
 print()
 return None
 # Prints only the names saved in the address book
 @staticmethod
 def saved():
 print('CONTACTS SAVED: ', end='')
 for j in range(0, len(address_book)):
 print(j, address_book[j].name, end=' || ')
 return None
 # Prompts the user if there's no contact saved in address book
 @staticmethod
 def isempty(list):
 if len(list) == 0:
 print('NO CONTACT SAVED\n')
 return True
 return False

The program now prompts the user for an option ranging from 0-5. Each option has a funcion in the address book: add, modify, delete, view, view all, finish.

address_book = []
msg_error = '{}Invalid option{}'.format('033円[31m', '033円[m' # Red text
access = input('Press any key to access')
while True:
 print('=-=-===-=-=-=-=- CONTACTS MENU -=-=-=-=-=-==-==')
 print("""[ 1 ] ADD [ 3 ] DELETE [ 5 ] VIEW ALL
[ 2 ] MODIFY [ 4 ] VIEW [ 0 ] FINISH""")
 option = input('>>> ')
 #This if and elif blocks check if user input ranges from 0-5
 if not option.isnumeric():
 print(msg_error)
 continue
 elif option not in '012345':
 print(msg_error)
 continue
 # If between 0-5, convert value to integer and...
 else:
 option = int(option)
 if option == 0:
 print('>>> Program ended\n')
 break
 # Add new contact
 elif option == 1:
 name = input('Name: ').capitalize().strip()
 surname = input('Surname: ').capitalize().strip()
 number = input('Number: ').strip()
 email = input('Email: ').strip().lower()
 # Trasnform into Contact class and append to address book
 address_book.append(Contact.add(name, surname, number, email))
 print('Contact saved\n')
 # Modify a contact
 elif option == 2:
 if Contact.isempty(address_book):
 continue
 Contact.saved()
 name_index = int(input('\nModify which name? '))
 print('\nModify which entry?')
 entry_index = int(input('[ 1 ] NAME [ 2 ] SURNAME [ 3 ] NUMBER [ 4 ] EMAIL\n>>>'))
 # Use object methods to modify within the list address book
 # User wants to modify name
 if entry_index == 1:
 modification = input('New name: ').capitalize().strip()
 address_book[name_index].edit_name(modification)
 # User wants to modify surname
 elif entry_index == 2:
 modification = input('New surname: ').capitalize().strip()
 address_book[name_index].edit_surname(modification)
 # User wants to modify number
 elif entry_index == 3:
 modification = input('New number: ').strip()
 address_book[name_index].edit_number(modification)
 # User wants to modify email
 elif entry_index == 4:
 modification = input('New email: ').lower().strip()
 address_book[name_index].edit_email(modification)
 print('Modification saved\n')
 # Delete a contact
 elif option == 3:
 if Contact.isempty(address_book):
 continue
 Contact.saved()
 name_index = int(input('\nWhich contact delete? '))
 del address_book[name_index]
 print('Contact deleted')
 # View specific contact details
 elif option == 4:
 if Contact.isempty(address_book):
 continue
 Contact.saved()
 index = int(input('\nContact position: '))
 Contact.summary(index)
 # View details of all contacts
 elif option == 5:
 if Contact.isempty(address_book):
 continue
 Contact.summary()
asked Nov 2, 2018 at 14:27
\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

You are using Python classes in a weird way.

  • First, if you want to create a new Contact instance, just call Contact(name, surname, number, email). No need for Contact.add, which does exactly the same thing.

  • Next, all of your static methods. They have (almost) nothing to do with the object Contact, they all operate on a list of contacts, the address book. This means you can either make them functions/inline code that operates on that list, or make an AddressBook class that handles it. In this case I would probably go for the first option.

    if Contact.isempty(address_book):
     continue
    

    becomes

    if not address_book:
     print('NO CONTACT SAVED\n')
     continue
    

    This uses the fact that empty lists are falsy. And Contact.saved becomes

    print('CONTACTS SAVED: ', end='')
    for j, contact in enumerate(address_book):
     print(j, contact.name, end=' || ')
    

    Where iterating over an iterable is preferred rather than iterating over the indices.

    You could (and probably should) put this into a function:

    def print_address_book(address_book):
     print('CONTACTS SAVED: ', end='')
     for j, contact in enumerate(address_book):
     print(j, contact.name, end=' || ')
    
  • All of your Contact.edit_* methods are not needed. In Python you usually want to avoid making getters and setters and instead use bare attributes unless you really have to. And you already have them being normal attributes, so you can directly modify them. But before we do that, it might make sense to allow dictionary like access to those entries, so you can use contact["name"] instead of contact.name:

    class Contact:
     ...
     def __getitem__(self, key):
     return getattr(self, key)
     def __setitem__(self, key, value):
     setattr(self, key, value)
    

    This makes updating a contact a bit easier:

    name_index = int(input('\nModify which name? '))
    while name_index not in range(len(address_book)):
     name_index = int(input('\nModify which name? '))
    print('\nModify which entry?')
    entry = input('NAME, SURNAME, NUMBER, EMAIL\n>>>').lower()
    while entry not in {"name", "surname", "number", "email"}:
     entry = input('NAME, SURNAME, NUMBER, EMAIL\n>>>').lower()
    address_book[name_index][entry] = input('New entry: ')
    print('Modification saved\n')
    

    I also added some code to ensure that the user inputs a sensible choice of input.

  • Choosing 0 to mean print all contacts in Contact.summary (which should also not be a static method but a stand alone function or method of AddressBook) is not a good idea. It means you cannot print the first contact (remember, Python starts counting at zero). Using -1 (as is sometimes done in C/C++) as a sign of printing all entries would also be a bad idea, because then you could not print the last contact (without calling len).

    Instead, just use None:

    try:
     index = int(input('\nContact position: '))
    except ValueError:
     index = None
    print_summary(address_book, index)
    def print_summary(address_book, index=None):
     if index is None:
     for contact in address_book:
     print(contact)
     else:
     print(address_book[index])
     print()
    

    As you can see I got rid of your explicit printing, instead make it part of the Contact class that when being printed it looks nice. For this you can use the magic method __str__, which should return a string representation of the instance:

    class Contact:
     ...
     def __str__(self):
     return " / ".join([self.name, self.surname, self.number, self.email])
    
  • This (as well as __getitem__ and __setitem__) is a magic (or dunder) method. Have a look here for what other special methods you can define to give your custom classes built-in behaviour: https://rszalski.github.io/magicmethods/

answered Nov 2, 2018 at 15:25
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.