User input fixes

[AlisterD]

Overview

User ID verification with the following criteria:

1. Maximum length of 25 letters or numbers
2. Minimum length of one letter or number
3. No special characters
4. No spaces

Added User ID Verification for the following

#176210324

https://www.pivotaltracker.com/story/show/175563946

Contributions

• Created function user_id_validator, imported regex to help with validation, and called function in start_experiment

Tested as much as I could with combinations oh spaces, special charts, numbers,

• ran make bci-gui for every case

[tab-cmd]

Nice addition!

[tab-cmd]

This method should go in the check_input method, which already extracts the self.user.

[AlisterD]

Oh that make sense! thank you for the feedback!

[tab-cmd]

As mentioned below this value should already be set when you call this in check_input(). In the current flow, we would extract user from the input and assign self.user twice.

[AlisterD]

deleted from function

[tab-cmd]

Great documentation!

[tab-cmd]

Consider assigning a length_user variable since you call it twice, and you could then append it in the message of the alert using f strings,

f'User ID must be of at least one or up to 25. Your current ID is {length_user} characters.'

[tab-cmd]

These comments are off.. try running make lint and see if it will catch this formatting stuff for you. If not, you'll need to go in and fix!

[tab-cmd]

We could add these as class-level variables at the top. For instance,

self.max_length = 25 in the init

and then use that value instead of the hardcoded 25. Update the documentation to reflect that we set it above. This will make changing it easier in the future. I'd do this for all cases. self.min_length, self.special_character_regex

[lawhead]

There is a lot of repeated code in this method that follows the same pattern: the user_id is checked against some condition and if it evaluates to True an alert is displayed with an error message specific to that condition. In addition to the increased code, this is also difficult to fully test and will only provide the user with 1 error message at a time. If the user_id has multiple problems this can be a frustrating experience for the end user.

One way to avoid repeating yourself is to abstract out the validations as separate functions (each of which can be independently tested), and store the validation functions along with the error message. For example:

# These functions could be in a separate module
def invalid_length(min=1, max=25):
    return lambda string: len(string) < min or len(string) > max

def contains_whitespaces(string):
    return re.match(r"^(?=.*[\s])", string)

def has_special_characters(string):
    disallowed_chars = re.compile('[^0-9a-zA-Z]+')
    return bool(disallowed_chars.search(string))

# This would go in the class definition
user_id_validations = [
  (invalid_length(min=self.min_length, max=self.max_length), f'must contain between {self.min_length} and {self.max_length} alphanumeric characters'),
  (contains_whitespaces, 'cannot contain white spaces'),
  (has_special_characters, 'cannot contain special characters') 
]

Then in the user_id_validator method you can iterate through these validations, check them against the user_id and collect the error messages for any that return True. If any problems are detected they can all be displayed to the user.

[AlisterD]

This is so elegant!

[AlisterD]

It looks great~