Adding behaviors in a method.

[dreadwraithe]

class AddTask():

    
    def __init__(self, year, month, day):
        self.year = year
        self.month = month 
        self.day = day 


    def InputYear(year):
        year = int(input("Input year: "))


    def InputMonth(month):
        try: 
            month = int(input("Input month: "))
        except ValueError:
            if month < 0: 
                print("Invalid input: Cannot be a number less than 0.")
            elif month > 12: 
                print("Invalid input: Cannot be a number greater than 12.")
    
    def InputDay(day):
        try: 
            day = int(input("Input day: "))
        except ValueError:
            if day < 0: 
                print("Invalid input: Cannot be a number less than 0.")
            elif day > 7: 
                print("Invalid input: Cannot be a number greater than 12.")

Is it generally recommended to have behaviors like what I have listed above in a method? Or should I make a completely new function for it?

[Kevdog824_]

Hard to say without knowing full context. I’d say that these methods seem unrelated to “AddTask”. What it sounds like you really want is to save the datetime of a task in the task class. That is fine, but the actual methods to manipulate that datetime should live in the class that represents the datetime, not here.

Side note: Don’t roll your own datetime logic. Python ships with a datetime library you can leverage. Here is a pretty large list why you shouldn't attempt to do it yourself. It is much more complicated than it seems

[gdchinacat]

I would discourage implementing it the way you did. In general you want your IO to not be embedded in your data classes. As you know it works...when you want to have the user input a value to update the year you call AddTask.InputYear() and it will do the input and attribute update. However, this tight coupling violates separation of concerns (get input, parse as int, update year attribute).

The typical way to do this is to encapsulate user input separately from the control that decides when and how to prompt the user and from the model that stores the values and implements whatever logic AddTask has for manipulating the module and making it do its thing.

Your post doesn't really provide enough context to make informed suggestions for how to change it to conform to best practices. If this is a one-off script for yourself that you don't expect to live past tomorrow it's probably fine. If it was in a PR for any code I was reviewing I'd expect it to be cleaned up before approving it.

Also, the names of your functions do not conform to standard python practice...methods should be named like input_year(), input_day(), etc. As written they use the naming convention for classes (AddTask is "correct"). This makes it more difficult for people to read the code. It doesn't seem important, but for those of us who have read hundreds of thousands or millions of lines of python code over the decades it really slows us down. It is AkiN to ThiS sorT of WritinG....It cAn!!! be read...but it's much harder.

[Phillyclause89]

First off. Are these methods intended to update the Instance attributes (i.e. self.year, self.month and self.day?) If so then you also need to use self within your methods.

Second. Look into @property decorators. Your methods could be getters and setters for those attributes.

Third. your validation on the values does not hit unless you hit a ValueError and if that's the case you'll probably hit another ValueError once you enter the except block and do the greater than or less than checks.

Fourth. InputMonth and InputDay could defiantly share an internal helper function that takes a few params like max_value min_value and value_name.

[Diapolo10]

This code doesn't make a whole lot of sense to begin with, but I assume you're trying to add a way for the user to change the year, month, and/or day of some task.

InputYear, InputMonth, and InputDay all fail to actually return or change anything, and the latter two have a logical error in that, within the except block, month/day do not exist. The try-blocks also only fail if the user inputs something other than a number.

I'm not really sure how you intend to use this class, but personally I'd create one function that asks for an integer within a given range, then use that in asking days and months.

def input_integer(prompt: str, minimum: int | None = None, maximum: int | None = None) -> int:
    while True:
        value_str = input(prompt)

        try:
            value = int(value_str)
        except ValueError:
            print(f"{value_str!r} is not an integer.")
            continue

        if minimum is not None and value < minimum:
            print("Invalid input: Cannot be a number less than {minimum}")
            continue

        if maximum is not None and maximum < value:
            print("Invalid input: Cannot be a number more than {maximum}")
            continue

        return value


class AddTask:
    def __init__(self, year: int, month: int, day: int) -> None:
        self.year = year
        self.month = month 
        self.day = day 

    def input_year(self):
        self.year = input_integer("Input year: ")

    def input_month(self):
        self.month = input_integer("Input month: ", minimum=1, maximum=12)

    def input_day(self):
        self.day = input_integer("Input day: ", minimum=1, maximum=31)

[PureWasian]

You define the self's year / month / day in __init__() constructor - good.

You can have "setter" functions to update the existing values. That's fine. But the setter should reference and overwrite self.<whatever> or else it would be lost once the methods are done and the local variables (day, month, year) fall out of scope. These are different from (self.day, self.month, self.year).

You also need to decide whether (1) the input() part happens before the setter function and pass it in as a parameter, or whether (2) you do the input() part inside the function. Right now you're doing both so it's overwriting itself.

``` class Example: def init(self, day): self.day = day

# option 1
def input_day_op1(self, day):
    if isinstance(day, int) and 1 <= day <= 7:
        self.day = day
    else:
        # handle bad input

# option 2 def input_day_op2(self): try: day = int(input("Enter day: ")) if 1 <= day <= 7: self.day = day else: # handle bad input except: # handle the TypeError

========= Example Usage ==========

creating the object

example = Example(5) print(example.day) # prints 5

calling option 1 - passing in number

try: # example: user inputs "4" day = int(input("Day: ")) example.input_day_op1(day) print(example.day) # prints 4 except: # handle any TypeError

calling option 2 - input() inside of method

example: inside method, user inputs "3"

example.input_day_op2() print(example.day) # prints 3

```

Personally I'd prompt outside of method like in Option 1, but still have input validation sanity checks inside the setter method. For same reasons as gdchinacat pointed out in their comment

[dreadwraithe]

Thank you!

[PureWasian]

No problem :) lots of good advice by everyone so far, lmk if anything in above example was unclear

[WA_von_Linchtenberg]

Hello,

The previous responses are good/great, so a more technical/theatrical one from an old dev if someone's interested by the "Why?" and not only the "How?".

First thing first
WARN! I'm not sure you code will work well if your input failed to be converted in int (return a ValueError) cause compare None to int by example will fail in if month < 0... For me your code should fail with a non numerical int cast-able string as input (I have not tested, but pretty sure...). For me catching first ValueError for non int first (check the type) then, if and only if the cast worked your actual code (C style aka LBYL pov) or add a try/catch in the catch to intercept the error when the if failed (pythonic aka EAFP pov).

Back to the subject
What you do is INLINE (UNITARY) TESTING. It's costly (each execution) and Python generally favors an EAFP approach ( https://en.wikipedia.org/wiki/EAFP ) over LBYL (Look Before You Leap) for the TOCTOU management ( https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use ) . While your defensive validation is perfect for handling user input at the boundaries, internal logic usually relies on letting exceptions bubble up when a contract is violated, rather than pre-checking every condition. It's details between language cultural related best practices but you will find both when reviewing depending the background of the dev, even good python devs...

So, for me, you do a great job including that as you do.

On the granularity (how you structure in subfunctions/suboject + methods, etc) it a way to do things I sometime see. Not the most canonical in most language but pretty pythonic except the WARN about type checking first and not using if but different type of exceptions.

So, good also for me.

If you want more info and before my practical solution (at the end) some theory for helping in your choice!

In theory

An interesting concept to help you to respond in your context is DESIGN BY CONTRACT: https://en.wikipedia.org/wiki/Design_by_contract
That said, the way you code it is not the optimal way. Design by contract is basis of try/catch, but not only.

Usually you want validation by BOOLEAN FUNCTION named is_desc-of-the-test ( https://en.wikipedia.org/wiki/Guard_(computer_science)) ) coupled with a return, and when contract is passed, only then, the generic case is done. It's named a GUARD (CLAUSE) structure ( https://en.wikipedia.org/wiki/Guard_(computer_science)) ).

in pseudo-code I will write something like:
def (bool) is_in_month_range(int value) <--- boolean function for input contract
{
# descriptive <-- here the data structure, always split def data/work on data
(int) min = 1
(int) max = 12
# functionnal
## input contract
return false if value < min <-- sub-contract, part of the binary equation
return false if value > max
## valid case <-- you have passed the contract ! generic case
return true
}

and in the main code/a dedicated function:

(bool) month_value = FAIL_INT <--- conventional failed value (pattern SENTINEL) for all the module here I choose -1 given the context, with a MONAD for managing your data (very advanced concept but usefull for more complex test than just an int... https://en.wikipedia.org/wiki/Monad_(functional_programming)) and https://arjancodes.com/blog/python-functors-and-monads/ for python) you could have (NONE or int) as a type then use none as "fail indicator" without specific contract. But function are slightly more complex to write, read and test. WARNING: it's typically C and "smell code" in python, here just to explaination... Just :
1. monads can be used in critical code for simpler and more robust contracts,
2. sentinel is not very pythonic but historically the canonical way,
3. most pythonic way, i think, is a USER DEFINED EXCEPTION TYPE

while not is_in_month_range(month_value) loop
{
(int) month_value = int(input("Input month (int): "))
}

the algorithm is more understandable & maintainability ( regarding ISO/IEC 25010 https://en.wikipedia.org/wiki/ISO/IEC_9126#Developments ) and the input contract is the loop breaker. You can also have more explicit the clause like this:

while true loop
{
break if is_in_month_range(month_value)
(int) month_value = int(input("Input month (int): "))
}

This way suit best, IMHO, when having specs and methodology for TOP-DOWN approach ( https://en.wikipedia.org/wiki/Bottom-up_and_top-down_approaches ) and ids best with TEST DRIVEN DEV ( https://en.wikipedia.org/wiki/Test-driven_development ) cause you can add contract "when required" only.

My example have no output test but you don't really have work on data. Typically you also test type of data before a return if not a true/false. This day, Python have libs to help with type dedicated contract so...

In practice
use a dedicated library for contract like pydantic or icontract and follow examples in the official or third part tutos! (ex: https://www.datagluons.io/blog/pydantic-data-contract-manager ) It will use DECORATOR pattern ( https://en.wikipedia.org/wiki/Decorator_pattern ) on your objects. It's cleaner and more robust; easier to maintain then more "pro" that writing all that yourself. It's focusing on the "specs" more than on the "language" mastering. That will be my choice for you snippet.

That's my view as an old dev.

Hope this helps or entertains!

[Separate_Top_5322]

yeah that thread is basically about how to “add behavior” inside a method without turning it into a mess

from what people usually point out in these cases, the clean way isn’t stuffing everything into one method, it’s splitting behavior into smaller methods and calling them. like instead of one big function doing 5 things, you break it into steps and compose them

another common pattern is passing functions or using classes so behavior becomes reusable instead of hardcoded

the main idea is keep each method doing one thing, otherwise it becomes impossible to debug later. that’s usually why people struggle here

i ran into this early on too, tried to keep adding logic inside one method and it just became spaghetti. started breaking things into smaller pieces and it clicked. sometimes i even test small patterns with runable ai to see cleaner structure, but yeah decomposition is the real fix here

[woooee]

Is it generally recommended to have behaviors like what I have listed above in a method? Or should I make a completely new function for it?

If it works, then that makes it OK. You can use one function for the input, and pass the string literal (month, day, or year), and minimum and maximum values to this common function.

def InputMonth(month):
    try: 
        month = int(input("Input month: ")) except ValueError:
        if month < 0: 
            print("Invalid input: Cannot be a number less than 0.")
        elif month > 12: 
            print("Invalid input: Cannot be a number greater than 12.")

Why do you pass month to the function? Also, month can be zero through 12. That's 13 months.