'Logical error: program writes even though the condition didn't meet

I am trying to read and write to file. In this example, I am trying to read the file user_admin.txt and check if the user's username who is requesting for registration exists in the file or not. If exists, it will pass(in here, shows a message) and if it doesn't then register the user in the given format (in str(self))

"""
    ***user_admin.txt contains(separate lines)***
    111124;;;zerox;;;32@3432#4$$
    344377;;;zer;;;32@3432#4$$
    424554;;;zero;;;32@3432#4$$
    544334;;;zeronda;;;32@8986#5$$
    655334;;;zIT;;;32@0000#8$$
    113324;;;Bazer;;;32@0000#4$$
"""

#jupyter cell 1

import os
import re

class Admin:

    path = "./data/result/user_admin.txt" 

    def __init__(self, id=-1, username="", password=""): # constructor
        self.id = id
        self.username = username
        self.password = password

    def __str__(self):
        return f"{self.id};;;{self.username};;;{self.password}"
    
    def register_admin(self):
        path = "./data/result/user_admin.txt"  # Destination of the user_admin.txt file
        
        with open(path, "r", encoding="utf-8") as r:  # r means read
            r_contents = r.read()
            user_match = re.findall(r'\d+;;;\w+;;;.*\n?', r_contents)
            
            with open(path, "a+", encoding="utf-8") as f:  # f means append and read
                if os.stat(path).st_size == 0:
                    f.write(f"{self.id};;;{self.username};;;{self.password}\n")
                elif os.stat(path).st_size != 0:
                    for i in range(len(user_match)):
                        if self.username not in user_match[i]:
                            f.write(f"{self.id};;;{self.username};;;{self.password}\n")
                        else:
                            print("You are not welcome here") # I will use "pass" in actual code
        


#jupyter cell 2
admin_two = Admin(113324,'Bazer','32@0000#4$$')
admin_two.register_admin()

#Juppyter cell 3
 with open(path, "r", encoding="utf-8") as r:  # f means append and read
 r_contents = r.read()
 print(r_contents)
 

The result of Cell 3 is:

11124;;;zerox;;;32@3432#4$$
344377;;;zer;;;32@3432#4$$ 
424554;;;zero;;;32@3432#4$$
544334;;;zeronda;;;32@8986#5$$ 
655334;;;zIT;;;32@0000#8$$
113324;;;Bazer;;;32@0000#4$$
113324;;;Bazer;;;32@0000#4$$
113324;;;Bazer;;;32@0000#4$$
113324;;;Bazer;;;32@0000#4$$
113324;;;Bazer;;;32@0000#4$$
113324;;;Bazer;;;32@0000#4$$

I don't understand what's the logical error causing this. I greatly appreciate your help :)



Solution 1:[1]

Divide and Conquer

Would split the register_admin into 3 functions instead:

  1. parse file to get users using regex
  2. find desired user
  3. register user at file

Solving here is the separation of concerns or single responsibility principle (SRP):

First search (find or not), then register - in separated logical units (functions or methods).

Search in a loop

You can search in all elements with for-loop. But keep track of your findings and exit early if needed.

Solving here is the boolean flag found together with early-exit break.

Code suggested

# first decompose the methods in Cell 1 (split into smaller problem-solutions)

# (1) parse users
# class method, thus no self
def parse_users(path): 
    user_matches = []
    with open(path, "r", encoding="utf-8") as r:  # r means read
        r_contents = r.read()
        user_matches = re.findall(r'\d+;;;\w+;;;.*\n?', r_contents)
    return user_matches

# (2) find in users  (the search logic in the loop was the issue)
def find_in_users(self, user_matches):
    # search in all elements of user_matches
    found = False  # use a flag, which will turn True as soon as was found
    for i in range(len(user_matches)):
        if self.username in user_match[i]:  # remove `not` to find 
            found = True
            break  # as soon as found we must exit the loop
    # out of the loop: test if not in elements of user_matches at all
    return found

# (3) register at file
def register_at(self, path)
    user_matches = parse_users(path) 
    found = find_in_users(self, user_matches)
    if not found:  # may have two reasons: (a) no user in file or file size 0, (b) searched user not found   
        # append it to the file            
        with open(path, "a+", encoding="utf-8") as f:  # f means append and read
            if os.stat(path).st_size == 0:
                f.write(f"{self.id};;;{self.username};;;{self.password}\n")
            elif os.stat(path).st_size != 0:        
                f.write(f"{self.id};;;{self.username};;;{self.password}\n")
    else:
        print("You are not welcome here") # I will use "pass" in actual code


# then later in Cell 2
admin_two = Admin(113324,'Bazer','32@0000#4$$')
admin_two.register_at(path)

# finally in Cell 3

Further issues and optimization

What matters: file-size or found?

Is it really important if the file is empty or not (if os.stat(path).st_size) ?

Result that matters for registration is:

  • just no admin was found

So we could simplify those lines:

            if os.stat(path).st_size == 0:
                f.write(f"{self.id};;;{self.username};;;{self.password}\n")
            elif os.stat(path).st_size != 0:        
                f.write(f"{self.id};;;{self.username};;;{self.password}\n")

to:

            f.write(f"{self.id};;;{self.username};;;{self.password}\n")

Find word in entire CSV line ?!

What if any user in the file has chosen the admin's name Bazer as password?

Consider this line:

424554;;;zero;;;Bazer

It will find the user searched as Bazer although the actual username is zero.

You could just extract the user-names by adjusting the regex with capture-groups. So the user_matches would be a list of user-names only like e.g. ['zer', 'zero'].

Or even better use a real CSV-parser module for Python.

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1