| bug, code review

The Bugs I Found by Code Review

PLS DONOT LEAVE SHIT FOR NEXT CODING!

I found many bugs by code review in the process of testing server projects (coding by Go/Php/Java).

Here I only show the typical bugs by python demo.

Performance: redundant interaction

Redundant DB Query

Bad:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
# badcode demo

import pymysql


def query(user_id: int):
    db = pymysql.connect(host='h', user='u',password='p', database='d')
    cursor = db.cursor()
    cursor.execute("SELECT name,email,status FROM user WHERE user_id=%d" % user_id)
    user_info = cursor.fetchone()
    db.close()
    return user_info


def operate(user_id: int):
    uinfo = query(user_id)  # redundant db query
    if not uinfo:
        return None, 'not found user'
    # DO SOMETHING
    return 'ok', None


def do(user_id: int):
    uinfo = query(user_id)
    if not uinfo:
        return 'not found'
    if uinfo.status == 1:
        return 'xxx'
    _, err = operate(user_id)
    if err:
        return err
    return None

Fix:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
# how to fix

def operate(user_info: dict):
    # DO SOMETHING WITH user_info
    return 'ok', None


def do(user_id: int):
    uinfo = query(user_id)
    if not uinfo:
        return 'not found'
    if uinfo.status == 1:
        return 'xxx'
    _, err = operate(uinfo)
    if err:
        return err
    return None

It`s easy to find the redundant query in the demo code, real project is more complex than demo. But complex is not the reason to coding easy and casual.

Redundant HTTP Request

Bad:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
# badcode demo

import requests

def query(user_id: int)
    user_info = requests.get('http://localhost:8000/user/%d' % user_id).json
    return user_info


def do(l: list)
    """l - eg: [{'file': 'xxx', 'user_id': 1}, {'file': 'yyy', 'user_id': 1}]
    """
    result = []
    for each in l:
        uinfo = query(each['user_id'])  # query every time whether user if same or not
        each['user_info'] = uinfo
        result.append(each)
    return result

Fix:

1
2
3
4
5
6
7
8
9
10
11
12
# how to fix

def do(l: list):
    uinfo_map = dict()
    for each in l:
        uinfo = uinfo_map.get(each['user_id'])
        if uinfo:
            each['user_info'] = uinfo
            continue
        uinfo = query(each['user_id'])
        each['user_info'] = uinfo
    return l

I found at least 3 times in projects. Pls use i/o with care.

Not Break in Loop

Bad:

1
2
3
4
5
6
7
8
9
10
11
12
# badcode demo

from typing import List


def do(user_id: int, users: List[dict]):
    uinfo = dict()
    for u in users:
        if u['user_id'] == user_id:
            uinfo = u  # should break
            # Fix: break
    return uinfo

Chaotic Config Data

Bad:

1
2
3
4
5
6
7
8
9
10
11
12
13
# badcode demo

# Config File
ProductsA = [1, 2, 3]
ProductsB = [4, 5, 6]

# Logic File
def do(p: int)
    if not p in ProductsA and not p in ProductsB:
        return 'c'
    return False

# When add ProductsD in config file without modify logic file, this logic will working with throuble.

The bug is real happend in project. And in most cases, developers make this bug is adding functions in service which was coded by another.

More

  • Data Not Consistent: The data in DB and Redis is not consistent;

  • Miss error from downstream service;

  • Keep useless code making service more and more mussy;