I was tasked with writing a python script to check if a passed ID number exists within a file. The file I was give contains roughly 25,000 unique ID numbers. An ID number is a unique 9 digit number.
I have a couple specific things for you to comment on if you decide to review this:
As it stands, I convert the
user_id
passed in theid_exists
to astr
. I do this because I need the ID number to be the same type when I read the ID's from the file. Is there an easier way to accomplish this?This is my first time using a PEP 484 type hint,
Generator
. Am I using it correctly?This is also my first time utilizing a generator in this fashion. I used a generator here because instead of loading the entire file into memory, storing in a list, then iterating over the list, I can read each line and can exit early if an ID is found. Is it good that I'm using it in this way, or is there an easier way to do this?
Using a helper function is a new concept to me. Am I utilizing this helper function in the most pythonic way?
Of course, any constructive criticism beyond these points is welcome. Thanks in advance.
"""
Ensures id number matches any within the ID file.
An ID number is a unique 9 digit number.
"""
import random
import time
from typing import Union, Generator
def id_exists(user_id: Union[int, str]) -> bool:
"""
Determines if the passed ID exists within the ID file. The
ID passed must be of length 9 exactly, and only contain numbers.
:param Union[int, str] user_id: User ID to check for existence
:return bool: True if ID exists in file, False otherwise
"""
# Generator[yield_type, send_type, return_type] #
def get_ids() -> Generator[str, None, None]:
"""
Helper function for `id_exists`.
Yields each line in the ID file.
:return Generator[str, None, None]: Lines in ID file.
"""
with open("ids.txt", "r") as file:
for line in file:
yield line.strip()
# Convert user_id to <class 'str'> if not already #
if type(user_id) == int:
user_id = str(user_id)
# Ensure user_id matches specifications #
if len(user_id) != 9 or not user_id.isdigit():
raise ValueError("ID should be 9 characters long and all digits.")
# Check ID against file and return result #
return any(user_id == stored_id for stored_id in get_ids())
1 Answer 1
One of:
- Only take a string or a number.
- Use
str
either way. - Work against a bespoke class.
Yes. But I find it better to just use
Iterator[str]
, unless you're using the coroutine aspects of it.Yeah that's fine. But:
- You can just use
in
rather thanany(...)
. - You don't need the function
get_ids
you can just useany
, like you are now.
- You can just use
- Don't use
type(...) == int
, useisinstance
. Validating
user_id
should probably happen somewhere else.I'll make a class as an example to show this.
class UserId:
__slots__ = ('id',)
id: int
def __init__(self, id: int) -> None:
if not self.valid_id(id):
raise ValueError("ID should be 9 characters long and all digits.")
self.id = id
@staticmethod
def valid_id(id: int) -> bool:
if not isinstance(id, int):
return False
str_id = str(id)
return (
len(str_id) == 9
and str_id.isdigit()
)
def exists(self) -> bool:
user_id = str(self.id)
with open('ids.txt') as f:
return any(user_id == line.strip() for line in f)
print(UserId(123456789).exists())
-
\$\begingroup\$ Hmm, I wonder why did you write the
return
fromvalid_id()
method like this:return (and len(str_id) == 9 and str_id.isdigit())
? Is it just a style thing or...? \$\endgroup\$Grajdeanu Alex– Grajdeanu Alex2020年01月29日 06:59:41 +00:00Commented Jan 29, 2020 at 6:59 -
\$\begingroup\$ @GrajdeanuAlex. Are you talking about the first and? If so, it's not meant to be there. Forgot to remove it when I moved
isinstance
out of the check \$\endgroup\$2020年01月29日 07:01:54 +00:00Commented Jan 29, 2020 at 7:01 -
\$\begingroup\$ yep. Thought that's a hidden feature that I haven't heard about \$\endgroup\$Grajdeanu Alex– Grajdeanu Alex2020年01月29日 07:02:54 +00:00Commented Jan 29, 2020 at 7:02
-
\$\begingroup\$ @GrajdeanuAlex. That would be one weird feature. But tbh, it did look nicer... \$\endgroup\$2020年01月29日 07:03:34 +00:00Commented Jan 29, 2020 at 7:03
user_id
have leading 0's? If it can, the usingstr()
for the conversion could cause problems. An f-string f"{user_id:0>9}" could be used to convert the integer and pad it with leading 0's. \$\endgroup\$