Relevant Background:
Greetings. This is my first attempt at using the OOP paradigm (only really used R
prior to this).
The project prompt came from JetBrains Academy
where I am currently working through the Python Developer
track. The code works as expected and was accepted for the particular stage I am on, but I have some specific questions that will hopefully improve the program.
Objective:
You should allow customers to create a new account in our banking system.
Program Details:
Once the program starts, you should print the menu:
1. Create an account
2. Log into account
0. Exit
In our banking system, the credit card number's IIN (first 6 digits) must be 400000.
We often see 16-digit credit card numbers today, but it’s possible to issue a card with up to 19 digits using the current system. In the future, we may see longer numbers becoming more common. In our banking system, the customer account number can be any, but it should be unique. And the whole card number should be 16-digit length.
If the customer chooses ‘Create an account’, you should generate a new card number which satisfies all the conditions described above. Then you should generate a PIN code that belongs to the generated card number. A PIN code is a sequence of any 4 digits. PIN should be generated in a range from 0000 to 9999.
If the customer chooses ‘Log into account’, you should ask them to enter their card information. Your program should store all generated data until it is terminated so that a user is able to log into any of the created accounts by a card number and its pin. You can use an array to store the information.
After all information is entered correctly, you should allow the user to check the account balance; right after creating the account, the balance should be 0. It should also be possible to log out of the account and exit the program.
Specific Questions:
I'd like to know how to have the code chunk below print just
Bye!
and notBye!
and thenNone
below it:elif account_balance_selection == "0": print("\n") print("Bye!") > Bye! > None should be: > Bye!
I am using the following code to generate and store the
credit card number
andpin number
.def create_account(self): credit_card_number = "400000" + format(randint(0000000000, 9999999999), '010d') pin_number = format(randint(0000, 9999), '04d') self.card_numbers.append(credit_card_number) self.pin_numbers.append(pin_number)
Is there a way to ensure that the numbers that are getting added to card_numbers
and pin_numbers
are unique? Either during the generation stage, or somehow prior to appending them to the their respective arrays?
Code:
from random import randint
class BankingSystem:
def __init__(self):
self.card_numbers = []
self.pin_numbers = []
self.iin = 400000
def main_welcom_screen(self):
print(
"1. Create an account\n"
"2. Log into account\n"
"0. Exit"
)
main_menu_selection = str(input())
if main_menu_selection == "1":
self.create_account()
if main_menu_selection == "2":
self.account_login()
if main_menu_selection == "0":
print("\n")
return "Bye!"
def create_account(self):
credit_card_number = "400000" + format(randint(0000000000, 9999999999), '010d')
pin_number = format(randint(0000, 9999), '04d')
self.card_numbers.append(credit_card_number)
self.pin_numbers.append(pin_number)
print("\n")
print("Your card has been created")
print("Your card number:")
print(credit_card_number)
print("Your pin number:")
print(pin_number)
print("\n")
self.main_welcom_screen()
def account_login(self):
print("\n")
print("Enter your card number:")
entered_card_number = int(input())
print("Enter your PIN:")
entered_pin_number = int(input())
if str(entered_card_number) in self.card_numbers and str(entered_pin_number) in
self.pin_numbers:
print("\n")
print("You have successfully logged in!")
self.account_balance()
else:
print("\n")
print("Wrong card number of PIN!")
print("\n")
self.main_welcom_screen()
def account_balance(self):
print(
"\n1. Balance\n"
"2. Log out\n"
"0. Exit"
)
account_balance_selection = str(input())
if account_balance_selection == "1":
print("\n")
print("Balance: 0")
self.account_balance()
elif account_balance_selection == "2":
print("\n")
print("You have successfully logged out!")
print("\n")
self.main_welcom_screen()
elif account_balance_selection == "0":
print("\n")
print("Bye!")
print(BankingSystem().main_welcom_screen())
2 Answers 2
As is somewhat common for beginner OOP code, it's not particularly OOP; it's an awkward wrapper around a collection of methods that are still procedural. Consider attempting the following:
- Make an
Account
class - Store the accounts as a dictionary of
Account
objects by card number, not separated arrays of cards and PINs - Saying "PIN number" is redundant ("personal identification number number")
iin
is never used; delete it- Make a convenience function to show a text menu and call an associated function reference
str(input())
is redundant;input
already returns a string- Call
input
with a prompt string, not a blank - It's
welcome
, notwelcom
- Use f-strings for your digit formatting code
- Do not use
randint
; userandrange
since it's more natural to express an exclusive maximum here - Do not use
randint
for the PIN; use thesecrets
module for cryptographic strength - You were printing
None
because of yourprint(BankingSystem().main_welcom_screen())
, which is meaningless because that function does not return anything - You have a critical bug where any combination of existing PIN and card number will allow login; but this should be a match to a specific card-PIN pair
- You have another bug where the calls out of
main_welcom_screen
recurse back tomain_welcom_screen
, so eventually you will blow your stack. Don't recurse here. - Consider adding validation for your menu choices
- Consider adding an anti-bruteforce hang after invalid PIN input
- Do NOT acquire a user's PIN via regular
input
, which exposes the PIN to over-the-shoulder; usegetpass
instead
Example implementation
from dataclasses import dataclass
from getpass import getpass
from random import randrange
from secrets import randbelow
from time import sleep
from typing import Dict, Tuple, Callable, ClassVar
class Menu:
MENU: ClassVar[Tuple[
Tuple[
str, Callable[['Menu'], bool]
], ...
]]
def screen(self):
prompt = '\n'.join(
f'{i}. {name}'
for i, (name, fun) in enumerate(self.MENU)
) + '\n'
while True:
choice = input(prompt)
try:
name, fun = self.MENU[int(choice)]
except ValueError:
print('Invalid integer entered')
except IndexError:
print('Choice out of range')
else:
if fun(self):
break
@dataclass
class Account(Menu):
card: str
pin: str
@classmethod
def generate(cls) -> 'Account':
return cls(
card=f'400000{randrange(1e10):010}',
pin=f'{randbelow(10_000):04}',
)
def dump(self):
print(
f"Your card number: {self.card}\n"
f"Your PIN: {self.pin}"
)
def balance(self):
print('Balance: 0')
def logout(self) -> bool:
print('You have successfully logged out!')
return True
def exit(self):
print('Bye!')
exit()
MENU = (
('Exit', exit),
('Balance', balance),
('Log out', logout),
)
class BankingSystem(Menu):
def __init__(self):
self.accounts: Dict[str, Account] = {}
def create_account(self):
account = Account.generate()
print('Your card has been created')
account.dump()
self.accounts[account.card] = account
def log_in(self):
for _ in range(3):
card = input('Enter your card number: ')
pin = getpass('Enter your PIN: ')
account = self.accounts.get(card)
if account is None or account.pin != pin:
print('Wrong card or PIN')
sleep(2)
else:
print('You have successfully logged in!')
account.screen()
break
def exit(self) -> bool:
print('Bye!')
return True
MENU = (
('Exit', exit),
('Create an account', create_account),
('Log into an account', log_in),
)
BankingSystem().screen()
-
\$\begingroup\$ Thank for this. I am looking forward to reviewing this in more detail. A lot to learn! \$\endgroup\$Eric– Eric2021年02月12日 23:50:13 +00:00Commented Feb 12, 2021 at 23:50
-
1\$\begingroup\$ But I always use my PIN number in the ATM machine.... \$\endgroup\$James K– James K2021年02月13日 13:44:19 +00:00Commented Feb 13, 2021 at 13:44
-
\$\begingroup\$ Your
Account
class is missing abalance
and the card numbers are not unique. Your interactive menu system is great, but it distracts from the actual exercise: write anAccount
class. Mixing data with the menu system is also not a good idea ("dataclass Account extends Menu"?) \$\endgroup\$danzel– danzel2021年02月13日 20:35:15 +00:00Commented Feb 13, 2021 at 20:35 -
\$\begingroup\$ @danzel The missing balance is deliberate: it's the exact same as what the OP posted. The same goes for uniqueness: I've deliberately left out the machinery that would be required to generate unique card numbers. As for mixing data with the menu, I agree; but - baby steps. \$\endgroup\$Reinderien– Reinderien2021年02月13日 20:49:27 +00:00Commented Feb 13, 2021 at 20:49
-
\$\begingroup\$ @Reinderien generating the card number inside the
Account
class is a design flaw since neither theAccount
class nor its instances should know of any other card number. It's up to the BankingSystem to choose a (new and unique) card number. \$\endgroup\$danzel– danzel2021年02月13日 21:06:14 +00:00Commented Feb 13, 2021 at 21:06
Is there a way to ensure that the numbers that are getting added to card_numbers and pin_numbers are unique? Either during the generation stage, or somehow prior to appending them to the their respective arrays?
AFAIK the card number does not have to be random, just unique. So I would generate the numbers in a sequential manner, and store them to a database with a unique constraint.
In SQL you could perform a SELECT MAX statement to retrieve the largest card number and increment the range, but watch out for race conditions (use database locks).
For this, you need some persistence. For a self-contained system (vs distributed systems used in the real world) using a small SQLite database would be a good option.
Generating a random card number like what you're doing offers no guarantee that it will be unique, especially since you don't check the number against the list of previously generated numbers. A collision is extremely unlikely, since the range is very large but the code is nonetheless flawed in this aspect.
Say that we stick to your approach but store the card number as integer rather than string. Then you can retrieve the maximum number with the max function:
max(self.card_numbers)
and increment it by 1 or 10 or whatever.
Could be something like:
def generate_card_number():
# list is empty, first number shall be 4000000000000000
if len(self.card_numbers) == 0:
credit_card_number = 4000000000000000
else:
# increment
credit_card_number = max(self.card_numbers) +1
self.card_numbers.append(credit_card_number)
# or return a value
return credit_card_number
As for the PIN number what you are doing seems reasonable. It has to be random but not unique, since it's perfectly conceivable more than one customer will share the same PIN, which is definitely unavoidable after a few thousand customers.
Note that in the real world the last digit in a credit card number is a check-digit generated according to the Luhn algorithm. So the generation rules are a bit more complicated actually.
-
\$\begingroup\$ Thank you very much for this perspective and insight. The next step in the project is to implement the Luhn algorithm! \$\endgroup\$Eric– Eric2021年02月14日 21:34:44 +00:00Commented Feb 14, 2021 at 21:34
-
\$\begingroup\$ Note that in the real world, some attacks (including social engineering ones) are easier if the numbers can be predicted. Like TCP sequence numbers, they may benefit from hashing in some secret data to make the numbers unguessable. \$\endgroup\$Toby Speight– Toby Speight2022年08月16日 16:56:22 +00:00Commented Aug 16, 2022 at 16:56