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()
\$\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/

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.