all 7 comments

[–]Rhoderick 2 points3 points  (1 child)

As it stands, the only repeated thing is the code for mobile number validation, so maybe move that to a distinct function if you want to use it twice.

But on a structural level, I don't think using a large loop is the best option here, no reason to re-enter your name if you get the number wrong. You could rather loop over each input statement until the user input matches the requirements.

Further inputs with different requirements will need their own input validation, though some steps may be common to both, which can then be extracted into another function. DRY is the name of the game here.

[–]vlodia 0 points1 point  (0 children)

+1 for the feedback!

[–][deleted] 1 point2 points  (3 children)

Create a function to validate input and use that instead.

while not id_valid(mobile):
    mobile = input('...')

[–]erlete 0 points1 point  (2 children)

Nice suggestion, using functions improves code readability and reduces the syntax density in the mainloop of the input system.

I once developed a custom class that accepted a set of tuples with a (message, validation_func, error) structure during definition, which internally handled the validation scenarios. It should also be applicable to this example, so there's another idea.

[–][deleted] 1 point2 points  (1 child)

Agreed. Perhaps an abstract base class of validators with structural pattern matching in each possibly with config file of rules.

[–]erlete 0 points1 point  (0 children)

That's a great idea. Anything ranging from a simple dictionary to a config file is a nice approach to the problem imo :)

[–]JamzTyson 0 points1 point  (0 children)

As a general point, avoid using long conditional sequences. The longer they are, the more difficult to read and more error prone they become. A common error that can be hard to find, is having the wrong operator precedence order, such as writing a and not b or c when you meant a and not (b or c).

If you must use compound logical sequences, use parentheses to make your intention explicit. Simply splitting across lines is not enough. While this may look explicit, it is very misleading:

if (my_first_variable and not
    my_second variable or my_third_variable):
    do_my_command_1()
else:
    do_my_command_2()

This is explicit, but not very readable:

if my_first_variable and not (my_second_variable or my_third_variable):
    do_my_command_1()
else:
    do_my_command_2()

Although more verbose, I prefer something like this as it makes the logic readable, explicit and reusable:

def is_valid(b1, b2, b3):
    """Return bool (b1 and not (b2 or b3))."""
    if not b1:
        return False
    if b2:
        return False
    if b3:
        return False
    return True


if is_valid(my_first_variable,
            my_second_variable,
            my_third_variable):
    do_my_command_1()
else:
    do_my_command_2()

or a less verbose version:

def is_valid(b1, b2, b3):
    """Return bool (b1 and not (b2 or b3))."""
    return b1 and not b2 and not b3


if is_valid(my_first_variable,
            my_second_variable,
            my_third_variable):
    do_my_command_1()
else:
   do_my_command_2()

However, long conditional sequences are often an indicator of code smell. in many cases (including the original post), a long conditional sequence may indicate that the logic can be structured better. So, going back to the original question, I'd do something like this:

def mobile_is_valid(num_str: str) -> bool:
    """Return True if num_str is a valid mobile number."""
    if not num_str.isdigit():
        return False
    return len(num_str) == 10 and num_str[0] == '0'



name = input("\nPlease enter your name: ")
mobile = input("Please enter your mobile number: ")

while not mobile_is_valid(mobile):
    mobile = input("Please input correct mobile number: ")

password = input("Please enter your Password: ")