Opinions always differ on matters like this, but your UserService class seems
like a severe case of over-engineering.
First, three of the methods seem to offer nothing very substantive:
desactivate_user() and reactive_user() just set boolean attributes on a
User instance; and is_active() just returns an attribute. I would encourage
you to simplify things: just operate on the user instance directly.
Second, the update_activity_tracking() does more stuff to the user instance,
but there’s no obvious reason not to move the method into the User class as well.
Third, the update_password() is a good example of a situation where you could
use a property, because you need to perform some calculations before storing
the attribute. Here’s the basic pattern:
class User:
def __init__(self, password):
# Initialize an under-the-hood attribute.
self._password = None
# Call the setter.
self.password = password
@property
def password(self):
# The getter just returns it.
return self._password
@property.setter
def password(self, new_password):
# The setter does the needed calculation.
self._password = get_hashed_password(new_password)
Fourth, the register_user() method is a natural fit for a classmethod in
the User class:
class User:
@classmethod
def register_user(cls, email, password):
# Python allows you to call functions in keyword style even if the
# function defines arguments as required positionals. You can use
# this feature to simplify the code (eliminate some temporary
# variables in this case) while still preserving code readability.
today = datetime.today().strftime('%Y-%m-%d')
return cls(
email,
password,
registration_date = today,
active = 1,
sign_in_count = 2,
current_sign_in_on = today,
last_sign_in_on = today,
)
The User.__init__() method has a large number of required parameters. Perhaps
your use case demands that, but you might give some thought to which of the
attributes are truly required in all usages. For example, the method
could default to using registration_date if the two sign-on dates
are not provided (as we do above in register_user()).
Regardless of that decision, if
testing and debugging make direct interaction with __init__() tedious, you
can add more conveniences to the class, again using the @classmethod
mechanism.
Managing user passwords is tricky business, at least in the real world. It’s
good that you store a hashed password rather than the plaintext, but I would
not include even the hashed password attribute in any kind of printing,
logging, etc. Adjust your User.__str__() method accordingly.
In UserRepository.add_user() you can simplify things. If you are testing for
truth, just do it directly (if user.active) rather than indirectly (if user.active == True). Also, bool values in Python are a subclass of int
(for example, 199 + True == 200), so you can delete the conditional logic entirely and
just do something like this:
def add_user(self, user):
# Long lines can be made more readable by formatting
# them in the style of pretty-printed JSON. Notice how
# easy this style is to read, scan visually, and edit flexibly.
sql = '''
INSERT INTO users (
email,
password,
is_active,
sign_in_count,
current_sign_in_on,
last_sign_in_on
)
VALUES (%s, %s, %s, %s, %s, %s)
'''.strip()
cursor = self.conn.cursor()
cursor.execute(
sql,
(
user.email,
user.password,
int(user.active),
user.sign_in_count,
user.current_sign_in_on,
user.last_sign_in_on,
)
)
return cursor.fetchall()
Your capitalization of the User variable within various methods is
inconsistent both with your own usages and with the norms in the wider
Python community: FooBar for class names and foo_bar for instance
variables is the most common approach.
DB operations can fail. If this is just an early learning exercise, you
can safely ignore the problem, but if your goal is to make the application
more robust, the operations should be wrapped in try-except structures,
as hinted at in your code comments.
In my experience, web applications like this will have various classes
representing the entities of interest (User, Book, Author, whatever). And
several of those classes will need DB operations: create, read, update, delete,
etc. And those DB operations will be fairly general, in the sense that if you
write a good method to update one kind of instance (eg, User), it can often be
made to do the same thing for other kinds of instances, provided that the
substantive classes (User, Book, etc) have attributes, properties, or methods
to deliver the information (things like table name and a list of (COL_NAME, ATTRIBUTE_VALUE) tuples) that a general-purpose DB method needs in order to
perform its operation. All of which is to say that UserRepository might
be too narrow of a focus. Instead, think about whether and how you might
convert that class into just Repository.
If this web application is intended to be a long-running application, you’ll
want to wire it up to use the logging framework at some point (rather than
just printing stuff), but that can come whenever you are ready to learn about
it.
Regarding your question about frequency of saving DB changes, don’t worry about
it or overthink it. Build your application with reasonable Python code — you
are off to a good start — and if your application ever needs to achieve higher
scale, solve the problem when it is clearly visible on the horizon. It’s often
easier to change DBs, scale outward (more running instances of your application
in different processes or on different servers), or alter the data schema than
it is to write tricky “save every N operations” code.
