# 一些程式碼的壞味道 (Bad Smells)

## 類別或方法過長
### [Long Method](https://refactoring.guru/smells/long-method)


* 簡介:
* 有個類的方法裡面邏輯很多/很雜亂/很長。
* 我們可能會想要盡量將邏輯拆分成多個專一的小邏輯，因為：
* 如果可以細看到大房子如何以每個負責不同功能的小積木組成，將有助於理解大房子的架構。
* 小積木也比較好去個別理解/修改/除錯。
* 缺點:
* 阻礙理解。
* 維護困難。
* 修改不易。
* 範例:
Before:

    import cv2
    import numpy as np


    class ImageRegistration:
        def __init__(
            self,
            fixed_image: np.ndarray,
            moving_image: np.ndarray
        ):
            self.fixed_image = fixed_image
            self.moving_image = moving_image

        def register_images(self):
            # Convert images to grayscale
            fixed_gray = cv2.cvtColor(
                self.fixed_image, cv2.COLOR_BGR2GRAY
            )
            moving_gray = cv2.cvtColor(
                self.moving_image, cv2.COLOR_BGR2GRAY
            )

            # Detect ORB features and compute descriptors
            orb = cv2.ORB_create()
            fixed_keypoints, fixed_descriptors = orb.detectAndCompute(
                fixed_gray, None
            )
            moving_keypoints, moving_descriptors = orb.detectAndCompute(
                moving_gray, None
            )

            # Match descriptors using Brute-Force Matcher
            bf = cv2.BFMatcher(cv2.NORM_HAMMING, crossCheck=True)
            matches = bf.match(fixed_descriptors, moving_descriptors)
            matches = sorted(matches, key=lambda x: x.distance)

            # Find the homography matrix
            src_pts = np.float32([
                fixed_keypoints[m.queryIdx].pt for m in matches
            ]).reshape(-1, 1, 2)
            dst_pts = np.float32(
                [moving_keypoints[m.trainIdx].pt for m in matches
            ]).reshape(-1, 1, 2)
            homography_matrix, _ = cv2.findHomography(
                dst_pts, src_pts, cv2.RANSAC, 5.0
            )

            # Warp the moving image to align with the fixed image
            (height, width) = self.fixed_image.shape[:2]
            registered_image = cv2.warpPerspective(
                self.moving_image, homography_matrix, (width, height)
            )

            return registered_image

After:
* 將 `register_images` 裡面的 sub-tasks 抽成獨立的方法。
* 較能夠一眼看出 `register_images` 的行為。

    class ImageRegistration:
        def __init__(
            self,
            fixed_image: np.ndarray,
            moving_image: np.ndarray
        ):
            self.fixed_image = fixed_image
            self.moving_image = moving_image

        def register_images(self):
            fixed_gray = self.to_gray(self.fixed_image)
            moving_gray = self.to_gray(self.moving_image)

            fixed_keypoints, fixed_descriptors = self.detect_features(
                fixed_gray
            )
            moving_keypoints, moving_descriptors = self.detect_features(
                moving_gray
            )

            matches = self.match_descriptors(
                fixed_descriptors, moving_descriptors
            )

            homography_matrix, _ = self.find_homography(
                fixed_keypoints, moving_keypoints, matches
            )

            (height, width) = self.fixed_image.shape[:2]
            registered_image = self.warp_image(
                self.moving_image, homography_matrix, width, height
            )

            return registered_image

        def to_gray(self, image: np.ndarray):
            return cv2.cvtColor(image, cv2.COLOR_BGR2GRAY)

        def detect_features(self, gray_image: np.ndarray):
            orb = cv2.ORB_create()
            keypoints, descriptors = orb.detectAndCompute(gray_image, None)
            return keypoints, descriptors

        def match_descriptors(
            self,
            fixed_descriptors: np.ndarray,
            moving_descriptors: np.ndarray
        ):
            bf = cv2.BFMatcher(cv2.NORM_HAMMING, crossCheck=True)
            matches = bf.match(fixed_descriptors, moving_descriptors)
            return sorted(matches, key=lambda x: x.distance)

        def find_homography(
            self,
            fixed_keypoints,
            moving_keypoints,
            matches
        ):
            src_pts = np.float32([
                fixed_keypoints[m.queryIdx].pt for m in matches
            ]).reshape(-1, 1, 2)
            dst_pts = np.float32([
                moving_keypoints[m.trainIdx].pt for m in matches
            ]).reshape(-1, 1, 2)
            return cv2.findHomography(
                dst_pts, src_pts, cv2.RANSAC, 5.0
            )

        def warp_image(
            self,
            image: np.ndarray,
            homography_matrix: np.ndarray,
            width: int,
            height: int
        ):
            return cv2.warpPerspective(
                image, homography_matrix, (width, height)
            )

---

## 物件導向濫用

### [Switch Statements](https://refactoring.guru/smells/switch-statements)

- [ ] Done


* 簡介:
* 冗長的 `switch` 或 `if-else` 判斷式
* 建議用 `polymorphism` 代替 `switch` 或 `if-else`語法

* 缺點:
* 可讀性降低、維護困難。

* 範例:
Before:

    def print_animal_sound(animal):
        if animal == "dog":
            print("Woof!")
        elif animal == "cat":
            print("Meow!")
        elif animal == "cow":
            print("Moo!")
        else:
            print("Unknown animal")

    print_animal_sound("dog")
    print_animal_sound("cat")
    print_animal_sound("cow")

After:

    class Animal:
        def print_sound(self):
            pass

    class Dog(Animal):
        def print_sound(self):
            print("Woof!")

    class Cat(Animal):
        def print_sound(self):
            print("Meow!")

    class Cow(Animal):
        def print_sound(self):
            print("Moo!")

    def print_animal_sound(animal):
        if isinstance(animal, Animal):
            animal.print_sound()
        else:
            print("Unknown animal")
    
    dog = Dog()
    cat = Cat()
    cow = Cow()

    print_animal_sound(dog)
    print_animal_sound(cat)
    print_animal_sound(cow)

## 防止改動

### [shutgun surgery](https://refactoring.guru/smells/shotgun-surgery)


* 範例:
Before:

    class Order:
        def __init__(self, total_price):
            self.total_price = total_price

        def apply_discount(self):
            if self.total_price > 1000:
                self.total_price *= 0.9

    class Customer:
        def __init__(self, age):
            self.age = age

        def apply_discount(self, order):
            if self.age > 65:
                order.total_price *= 0.85

    # 可以根據訂單價格，或是客戶等級，來決定如何打折。

After:

    class Discount:
        @staticmethod
        def apply_order_discount(total_price):
            if total_price > 1000:
                return total_price * 0.9
            return total_price

        @staticmethod
        def apply_customer_discount(age, total_price):
            if age > 65:
                return total_price * 0.85
            return total_price

    class Order:
        def __init__(self, total_price):
            self.total_price = total_price

        def apply_discount(self):
            self.total_price = Discount.apply_order_discount(self.total_price)

    class Customer:
        def __init__(self, age):
            self.age = age

        def apply_discount(self, order):
            order.total_price = Discount.apply_customer_discount(self.age, order.total_price)

* 心得:
* 因為打折邏輯可能會一直變動。因此乾脆把容易更動的打折邏輯聚集，到`打折`類。如此，其他類就可以減少被改動的機會。

## 過分耦合

過度耦合的跡象: 牽一髮動全身; 改來改去改來改去。==What the F...? 為何改了`客戶類`還不夠，其他像是`訂單類`，`工廠類`，明明感覺是分開的東西，卻還得隨同一起改動？==

### [Feature Envy](https://refactoring.guru/smells/feature-envy)

* 範例:
Before:

    class Customer:
        def __init__(self, first_name, last_name, age):
            self.first_name = first_name
            self.last_name = last_name
            self.age = age

        def full_name(self):
            return f"{self.first_name} {self.last_name}"


    class Order:
        def __init__(self, customer, total_amount):
            self.customer = customer
            self.total_amount = total_amount

        def get_customer_age(self):
            return self.customer.age

        def get_customer_full_name(self):
            return self.customer.full_name()

        def is_eligible_for_discount(self):
            return self.get_customer_age() > 60
        # 要看客戶的某特殊特性，以決定訂單打折與否
        # 因為是設計在客戶自身的打折條件，可以把此方法改放置於客戶類別，而非訂單類別。

After:

    class Customer:
        def __init__(self, first_name, last_name, age):
            self.first_name = first_name
            self.last_name = last_name
            self.age = age

        def full_name(self):
            return f"{self.first_name} {self.last_name}"

        def is_eligible_for_discount(self):
            return self.age > 60


    class Order:
        def __init__(self, customer, total_amount):
            self.customer = customer
            self.total_amount = total_amount

        def is_eligible_for_discount(self):
            return self.customer.is_eligible_for_discount()
        # 訂單類調用客戶類，請客戶類告知該客戶是否要打折。
        # 未來打折邏輯若是改變，訂單類將不需要改動。因為訂單類不需要知道打折的實作細節。

* 心得: 若打折的方法改為: 參考客戶是否為VIP, 是的話，打折。則要同時修改客戶類和訂單類(客戶追加是否VIP, 訂單類追加檢查客戶是否為VIP)。但經過重構，程式碼去耦合後，便只需要修改客戶類即可。
* 簡介
==某類的方法中，常常拿他類的方法/資料去做事情。該類覬覦其他類別的功能或資料，將其他類的方法或資料，加以整合成為自己內部的一部分。==
* 缺點
缺乏維護性。改動某一小功能時，被迫要於多個類裡面去做改動，而不是只要在單一類別內改動就好。

### [**Message Chains**](https://refactoring.guru/smells/message-chains)


* 範例
Before:

    class Wheel:
        def __init__(self, diameter):
            self.diameter = diameter

        def get_diameter(self):
            return self.diameter


    class Bicycle:
        def __init__(self, wheel):
            self.wheel = wheel

        def get_wheel(self):
            return self.wheel


    class Cyclist:
        def __init__(self, bicycle):
            self.bicycle = bicycle

        def get_bicycle(self):
            return self.bicycle


    class RaceOrganizer:
        def __init__(self, racer):
            self.racer = racer

        def get_wheel_diameter(self):
            return self.racer.get_bicycle().get_wheel().get_diameter()
            # 寫了太多不必要細節。 比賽主辦者應該只需要請騎士給予車輪的半徑。
            # 不需要請騎士叫他的特定車子(腳踏車)，去取得輪子，然後請該輪子提供半徑。

        # 這樣寫的話，未來這個類被改動的機會很大。因為哪天如果換成
        # 汽車比賽，飛行器比賽等，這邊的程式碼就要改動了: 
        #   self.racer.get_car().get_wheel().get_diameter()
        #   self.racer.get_flying_object().get_landing_wheel().get_diameter()

After:

    class Wheel:
        def __init__(self, diameter):
            self.diameter = diameter

        def get_diameter(self):
            return self.diameter
            # 我是輪子，我有自己輪胎直徑的資訊

    class Bicycle:
        def __init__(self, wheel):
            self.wheel = wheel

        def get_wheel_diameter(self):
            return self.wheel.get_diameter()
            # 這個腳踏車可去檢查自己的輪子，藉此取得輪胎直徑

    class Cyclist:
        def __init__(self, bicycle):
            self.bicycle = bicycle

        def get_wheel_diameter(self):
            return self.bicycle.get_wheel_diameter()
            # 這個腳踏車騎士可從他的腳踏車去找輪胎直徑

    class RaceOrganizer:
        def __init__(self, racer):
            self.racer = racer

        def get_wheel_diameter(self):
            return self.racer.get_wheel_diameter()
        # 比賽主辦方可以問比賽者要輪子直徑資訊，但不需要知道
        # 或是不需要指定比賽者怎麼去取得該資訊的
        # (不用知道比賽者是使用什麼交通工具/ 交通工具是否有輪子能夠被測量出直徑)

* 謎之聲
==A類調用B類，B類又去調用C類...。很明顯的，A類不去**直接**使用C，而是**間接**的去使用C。
試問，A類會不會只要能夠調用B即可。A真的有間接去使用C的必要嗎？==
* 缺點
不好復用。未來改動時，很多程式碼可能會需要重寫。

## 垃圾太多
### Comments (過多的註解)

* 範例
Before

    def find_largest(numbers):
        # Create a variable to store the largest number
        largest_number = None

        # Iterate through the list of numbers
        for number in numbers:
            # Check if the current number is greater than the largest number
            if largest_number is None or number > largest_number:
                # Update the largest number
                largest_number = number

        # Return the largest number
        return largest_number


    # Create a list of numbers
    numbers = [5, 3, 8, 1, 10, 7]

    # Call the find_largest function with the list of numbers
    result = find_largest(numbers)

    # Print the largest number
    print("The largest number is:", result)

After:

    def find_largest(numbers):
        largest_number = None
        for number in numbers:
            if largest_number is None or number > largest_number:
                largest_number = number
        return largest_number


    numbers = [5, 3, 8, 1, 10, 7]
    result = find_largest(numbers)
    print("The largest number is:", result)

* 謎之聲
註解是好事; 可是如果很多註解，有可能是因為程式碼寫的太恐怖，不好懂，才需要很多註解。