Phần 2:https://techmaster.vn/posts/34618/xu-ly-code-lom-phan-2
Trong loạt bài này tôi sẽ nói về một số ví dụ khi code lởm, tin tôi đi bạn cũng đã từng như thế….Mục đích bài viết này chỉ là làm thế nào để viết code dễ hiểu hơn, dễ mở rộng và maintain, đúng thật là maintain nhiều dự án mới thấy anh em việt nam code vẫn ngon chán.
Loạt bài viết này các bạn cần biết về:
Swift
Dependency injection, factory method pattern và strategy pattern.
Note:
Tôi sẽ không giải thích về ngôn ngữ Swift, bạn có thể tìm đọc ở google nhé.
Các ví dụ sẽ thật đơn giản, với tôi thì cách học qua đọc code hiệu quả hơn nhiều so với cầm quyền sách đọc.
Với cả cách giải quyết vấn để của tôi k phải là cách duy nhất hay tốt nhất, đơn giản chỉ là cách đó sẽ tốt hơn cho mình và người khác có thể hiểu được.
Tôi cũng không quan tâm về các lỗi biên dịch đâu nhé :)), mình đang bàn luận về cách cải tiến code thôi.
class Tea1
{
func calculate(amount: Double, type: Int, years: Int) -> Double{
var result: Double = 0
let disc = (years > 5) ? Double(5/100) : Double(years/100)
if (type == 1)
{
result = amount
}else if (type == 2)
{
result = (amount - (0.1 * amount)) - disc * (amount - (0.1 * amount))
}else if (type == 2)
{
result = (0.7 * amount) - disc * (0.7 * amount)
}else if (type == 2)
{
result = (amount - (0.5 * amount)) - disc * (amount - (0.5 * amount))
}
return result
}
}
Đã rất nhiều lần tôi đọc được những đoạn code tựa như trên, thề luôn muốn tìm thằng viết vả cho mấy phát, không biết nó đang tính toàn cái gì, không sao chúng ta sẽ giải quyết từ từ đoạn mã thối này.
Trông có vẻ nó là giảm giá cái gì đấy đúng không, mà thật sự đoạn mã này quá khó đọc, khó maintain, mở rộng cũng khó vì k hiểu logic nó như nào, nhưng mình cũng đồng cảm vì trước lúc đi học cũng thường code kiểu như này
Vấn đề ở đây là gì?
- Tên:
Việc đặt tên rất quan trọng, ở đây tôi chỉ biết nó là tính toàn nhưng k biết cụ thể là gì việc ngồi đọc như này rất lãng phí thời gian không có comment thì khả năng sau này chính người viết cũng k hiểu để thêm các trường hợp để discount, mà tôi khuyên các bạn nên refactor lại hãy nghĩ cho người tiếp theo sau bạn đọc được những đoạn mã này, họ sẽ phải tốn thời gian như bạn hoặc nhiều hơn vì có thể bạn sẽ thêm logic vào nếu vậy thì bạn nên tự tát vào mặt mình 1 cái đi nhé…
- Hard-code:
Ở ví dụ này có biến type có kiểu int và nó được dùng để so sánh if else các kiểu từ đó để quyết định sử dụng logic nào để tính toán. Như này có thánh hiểu, cái ông nào viết kiểu này bảo đảm 1 tuần sau đọc chắc cũng đơ luôn.
- Không bắt lỗi:
Nghĩa là sao, ở đây nó tính toàn giá, oke nó có bắt default nhưng nếu vào default thì trả về 0, thể có nghĩa là mặt hàng đó miễn phí à….
- Khó đọc:
Bạn có đồng ý là nó khó đọc không.
- Lại hard-code:
0.1, 0.7, 0.5 không hiểu nó là gì với cái này nữa: result = (amount - (0.5m * amount)) - disc * (amount - (0.5m * amount)) => Nó là gì vậy.
- Đừng để lặp code:
Class này làm quá nhiều tác vụ(bạn có thể tham khảo quá SOLID partern nhé):Tính toàn tùm lum theo cả trạng thái tài khoản(type) và theo năm. Nghĩa là sao, sau tôi muốn bỏ cái tính theo type thì cái tính theo năm cũng không đúng nữa.
Cấu trúc lại code:
Đặt tên lại các biến:
Hãy đặt tên sao cho sao cho dễ đọc hơn như sau, ở đây chúng ta sẽ đổi luôn tên class:
class DiscountManager
{
func calculate(price: Double, accountStatus: Int, timeHavingAccountInYears: Int) -> Double{
var priceAfterDiscount: Double = 0
let discountForLoyaltyPercentage = (timeHavingAccountInYears > 5) ? Double(5/100) : Double(timeHavingAccountInYears/100)
if (accountStatus == 1)
{
priceAfterDiscount = price
}else if (accountStatus == 2)
{
priceAfterDiscount = (price - (0.1 * price)) - discountForLoyaltyPercentage * (price - (0.1 * price))
}else if (accountStatus == 2)
{
priceAfterDiscount = (0.7 * price) - discountForLoyaltyPercentage * (0.7 * price)
}else if (accountStatus == 2)
{
priceAfterDiscount = (price - (0.5 * price)) - discountForLoyaltyPercentage * (price - (0.5 * price))
}
return priceAfterDiscount
}
}
Nhưng vẫn còn 1,2,3,4 không biết nó là gì…
Hard-code:
Ngôn ngữ nào cũng thế thôi, bạn có thể tận dụng enum để giải quyết vấn đề này
enum AccountStatus: NSInteger
{
NotRegistered = 1,
SimpleCustomer = 2,
ValuableCustomer = 3,
MostValuableCustomer = 4
}
class DiscountManager
{
func calculate(price: Double, accountStatus: Int, timeHavingAccountInYears: Int) -> Double{
var priceAfterDiscount: Double = 0
let discountForLoyaltyPercentage = (timeHavingAccountInYears > 5) ? Double(5/100) : Double(timeHavingAccountInYears/100)
if (accountStatus == AccountStatus.NotRegistered)
{
priceAfterDiscount = price
}else if (accountStatus == AccountStatus.SimpleCustomer)
{
priceAfterDiscount = (price - (0.1 * price)) - discountForLoyaltyPercentage * (price - (0.1 * price))
}else if (accountStatus == AccountStatus.ValuableCustomer)
{
priceAfterDiscount = (0.7 * price) - discountForLoyaltyPercentage * (0.7 * price)
}else if (accountStatus == AccountStatus.MostValuableCustomer)
{
priceAfterDiscount = (price - (0.5 * price)) - discountForLoyaltyPercentage * (price - (0.5 * price))
}
return priceAfterDiscount
}
}Cải thiện đọc hiểu code:
Đầu tiên tối sẽ thay if else bằng switch case, tiếp theo đoạn mã nào dài quá thì cho nó xuống dòng để dễ đọc hơn ví dụ:
class DiscountManager
{
func calculate(price: Double, accountStatus: Int, timeHavingAccountInYears: Int) -> Double{
var priceAfterDiscount: Double = 0
let discountForLoyaltyPercentage = (timeHavingAccountInYears > 5) ? Double(5/100) : Double(timeHavingAccountInYears/100)
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price
break
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (0.1 * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.ValuableCustomer:
priceAfterDiscount = (0.7 * price)
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (0.5 * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
}
return priceAfterDiscount
}
}
Bắt lỗi tiềm ẩn:
Method ApplyDiscount sẽ return về 0 nếu k có trạng thái nào phù hợp, thế thì vỡ mặt. Vậy làm thế nào, với tôi thì tôi sẽ ném ra 1 exception
class DiscountManager
{
func calculate(price: Double, accountStatus: Int, timeHavingAccountInYears: Int) -> Double{
var priceAfterDiscount: Double = 0
let discountForLoyaltyPercentage = (timeHavingAccountInYears > 5) ? Double(5/100) : Double(timeHavingAccountInYears/100)
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price
break
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (0.1 * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.ValuableCustomer:
priceAfterDiscount = (0.7 * price)
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (0.5 * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
default:
throw NSError(domain: "error description", code: 42, userInfo: nil )
}
return priceAfterDiscount
}
}
Phân tích phần tính toán:
Trọng hàm tính toán này có 2 phần chính là tính theo status và year. Ở đây các bạn thấy có các logic như sau nhìn theo 1 quy tắc nhất định riêng có thằng này (0.7 * price) thì khác vậy ở đây tôi sẽ chuyển lại để cho nó có cùng logic:
class DiscountManager
{
func calculate(price: Double, accountStatus: Int, timeHavingAccountInYears: Int) -> Double{
var priceAfterDiscount: Double = 0
let discountForLoyaltyPercentage = (timeHavingAccountInYears > 5) ? Double(5/100) : Double(timeHavingAccountInYears/100)
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price
break
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (0.1 * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.ValuableCustomer:
priceAfterDiscount = price - (0.3 * price)
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (0.5 * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
}
return priceAfterDiscount
}
}
Sau khi đổi lại xong thì toan bộ logic sẽ là giá - (phần trăm * giá)
Tránh việc sử dụng hard-code(0.1, 0.3….)
Các số phần trăm giảm giá kia nhìn hơi nguy hiểm nhỉ, ở đây chúng ta đá có 1 biến tương tự
let discountForLoyaltyInPercentage = (timeOfHavingAccountInYears > 5) ? Double(5/100) : Double(timeOfHavingAccountInYears/100)
Số 5 nhìn có vẻ cũng nguy hiểm, giờ mình cần làm cho nó dễ hiểu hơn, ở đây tôi sẽ định nghĩa riêng các số này ra 1 class khác.
class Constants
{
static let MAXIMUM_DISCOUNT_LOYALTY: Int = 5
static let DISCOUNT_SIMPLE_CUSTOMERS: Int = 0.1
static let DISCOUNT_VALUABLE_CUSTOMERS: Int = 0.3
static let DISCOUNT_MOST_VALUABLE_CUSTOMERS: Int = 0.5
}
Và chỉnh lại class DiscountManager như sau:
class DiscountManager
{
func calculate(price: Double, accountStatus: Int, timeHavingAccountInYears: Int) -> Double{
var priceAfterDiscount: Double = 0
let discountForLoyaltyPercentage = (timeHavingAccountInYears > 5) ? Double(Constants.MAXIMUM_DISCOUNT_LOYALTY/100) : Double(timeHavingAccountInYears/100)
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price
break
case AccountStatus.SimpleCustomer:
priceAfterDiscount = (price - (Constants.DISCOUNT_SIMPLE_CUSTOMERS * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.ValuableCustomer:
priceAfterDiscount = (Constants.DISCOUNT_VALUABLE_CUSTOMERS * price)
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = (price - (Constants.DISCOUNT_MOST_VALUABLE_CUSTOMERS * price))
priceAfterDiscount = priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
break
}
return priceAfterDiscount
}
}
Giờ thì nhìn đoạn mã cũng dễ hiều hơn rồi nhể.
Không viết các đoạn mã lặp nhau:
- Ở đây các bạn có thể thấy các đoạn mã logic bị lặp nhau vây sau này khi muốn sửa đổi ở công thức tình toán chúng ta sẽ phải sử ở nhiều nơi, ở đây mình sẽ dùng extension để giải quyết vấn đề này:
extension Double
{
private func applyDiscountForStatus(discountSize: Double) -> Double
{
return price - (discountSize * self)
}
func applyDiscountForTime(discountSize: Double, timeHavingAccountInYears: Int) -> Double
{
let priceAfterDiscount = self.applyDiscountForStatus(discountSize: discountSize)
let discountForLoyaltyPercentage = (timeHavingAccountInYears > 5) ? Double(Constants.MAXIMUM_DISCOUNT_LOYALTY/100) : Double(timeHavingAccountInYears/100)
return priceAfterDiscount - (discountForLoyaltyPercentage * priceAfterDiscount)
}
}
class DiscountManager
{
func calculate(price: Double, accountStatus: Int, timeHavingAccountInYears: Int) -> Double{
var priceAfterDiscount: Double = 0
switch (accountStatus)
{
case AccountStatus.NotRegistered:
priceAfterDiscount = price
break
case AccountStatus.SimpleCustomer:
priceAfterDiscount = price.applyDiscountForTime(discountSize: Constants.DISCOUNT_SIMPLE_CUSTOMERS, timeHavingAccountInYears: timeHavingAccountInYears)
break
case AccountStatus.ValuableCustomer:
priceAfterDiscount = price.applyDiscountForTime(discountSize: Constants.DISCOUNT_VALUABLE_CUSTOMERS, timeHavingAccountInYears: timeHavingAccountInYears)
break
case AccountStatus.MostValuableCustomer:
priceAfterDiscount = price.applyDiscountForTime(discountSize: Constants.DISCOUNT_MOST_VALUABLE_CUSTOMERS, timeHavingAccountInYears: timeHavingAccountInYears)
break
}
return priceAfterDiscount
}
}
Tóm lại:
Vậy là phần1 này đã kết thúc ở phần tiếp theo chúng ta sẽ tiếp tục áp dụng Dependency injection, factory method pattern và strategy pattern để làm cho code nó nguy hiểm hơn.
Cảm ơn bạn đã đọc!
Bình luận