+8

SOLID - Đơn Nhiệm (P1)

Bài SOLID là thay đổi đã lướt qua một chút các nguyên tắc trong SOLID. Bài này chúng ta cùng bàn về nguyên tắc đầu tiên Single Responsibility - Đơn Nhiệm.

Mỗi class chỉ nên có duy nhất một lý do để thay đổi

Đây là một nguyên tắc đơn giản và tôi nghĩ ai cũng hiểu, nhưng nó lại nguyên tắc hay bị vi phạm và dễ vi phạm nhất. Dù đơn giản nhưng có vẻ nó thực sự khó. Dù bạn là người mới hay lão làng, làm trong công ty nhỏ hay công ty to, không có gì đảm bảo rằng bạn sẽ không viết ra những thứ khó hiểu. Để tôi cho các bạn ví dụ từ một bộ mã nguồn của Microsoft. Đây là một project mẫu xây dựng một trang ecommerce bằng dotnet do team dotnet viết.

Hãy xem class ProfileService này: https://github.com/dotnet/eShop/blob/main/src/Identity.API/Services/ProfileService.cs. Vì tôi không tìm thấy IProfileService ở đâu trong thư mục, nên tôi dùng tạm cả concrete class.

namespace eShop.Identity.API.Services
{
    public class ProfileService : IProfileService
    {
        private readonly UserManager<ApplicationUser> _userManager;

        public ProfileService(UserManager<ApplicationUser> userManager)
        {
            _userManager = userManager;
        }

        public async Task GetProfileDataAsync(ProfileDataRequestContext context)
        {
            var subject = context.Subject ?? throw new ArgumentNullException(nameof(context.Subject));

            var subjectId = subject.Claims.Where(x => x.Type == "sub").FirstOrDefault()?.Value;

            var user = await _userManager.FindByIdAsync(subjectId);
            if (user == null)
                throw new ArgumentException("Invalid subject identifier");

            var claims = GetClaimsFromUser(user);
            context.IssuedClaims = claims.ToList();
        }

        public async Task IsActiveAsync(IsActiveContext context)
        {
            var subject = context.Subject ?? throw new ArgumentNullException(nameof(context.Subject));

            var subjectId = subject.Claims.Where(x => x.Type == "sub").FirstOrDefault()?.Value;
            var user = await _userManager.FindByIdAsync(subjectId);

            context.IsActive = false;

            if (user != null)
            {
                if (_userManager.SupportsUserSecurityStamp)
                {
                    var security_stamp = subject.Claims.Where(c => c.Type == "security_stamp").Select(c => c.Value).SingleOrDefault();
                    if (security_stamp != null)
                    {
                        var db_security_stamp = await _userManager.GetSecurityStampAsync(user);
                        if (db_security_stamp != security_stamp)
                            return;
                    }
                }

                context.IsActive =
                    !user.LockoutEnabled ||
                    !user.LockoutEnd.HasValue ||
                    user.LockoutEnd <= DateTime.UtcNow;
            }
        }

        private IEnumerable<Claim> GetClaimsFromUser(ApplicationUser user)
        {
            var claims = new List<Claim>
            {
                new Claim(JwtClaimTypes.Subject, user.Id),
                new Claim(JwtClaimTypes.PreferredUserName, user.UserName),
                new Claim(JwtRegisteredClaimNames.UniqueName, user.UserName)
            };

            if (!string.IsNullOrWhiteSpace(user.Name))
                claims.Add(new Claim("name", user.Name));

            if (!string.IsNullOrWhiteSpace(user.LastName))
                claims.Add(new Claim("last_name", user.LastName));

            if (!string.IsNullOrWhiteSpace(user.CardNumber))
                claims.Add(new Claim("card_number", user.CardNumber));

            if (!string.IsNullOrWhiteSpace(user.CardHolderName))
                claims.Add(new Claim("card_holder", user.CardHolderName));

            if (!string.IsNullOrWhiteSpace(user.SecurityNumber))
                claims.Add(new Claim("card_security_number", user.SecurityNumber));

            if (!string.IsNullOrWhiteSpace(user.Expiration))
                claims.Add(new Claim("card_expiration", user.Expiration));

            if (!string.IsNullOrWhiteSpace(user.City))
                claims.Add(new Claim("address_city", user.City));

            if (!string.IsNullOrWhiteSpace(user.Country))
                claims.Add(new Claim("address_country", user.Country));

            if (!string.IsNullOrWhiteSpace(user.State))
                claims.Add(new Claim("address_state", user.State));

            if (!string.IsNullOrWhiteSpace(user.Street))
                claims.Add(new Claim("address_street", user.Street));

            if (!string.IsNullOrWhiteSpace(user.ZipCode))
                claims.Add(new Claim("address_zip_code", user.ZipCode));

            if (_userManager.SupportsUserEmail)
            {
                claims.AddRange(new[]
                {
                    new Claim(JwtClaimTypes.Email, user.Email),
                    new Claim(JwtClaimTypes.EmailVerified, user.EmailConfirmed ? "true" : "false", ClaimValueTypes.Boolean)
                });
            }

            if (_userManager.SupportsUserPhoneNumber && !string.IsNullOrWhiteSpace(user.PhoneNumber))
            {
                claims.AddRange(new[]
                {
                    new Claim(JwtClaimTypes.PhoneNumber, user.PhoneNumber),
                    new Claim(JwtClaimTypes.PhoneNumberVerified, user.PhoneNumberConfirmed ? "true" : "false", ClaimValueTypes.Boolean)
                });
            }

            return claims;
        }
    }
}

Các câu hỏi:

  • Task IsActiveAsync(IsActiveContext context) có nên ở trong IProfileService?
  • Đặt tên sai quy ước. IsActive thì nên trả về boolean, nếu không nó nên là CheckActiveContext (vì thế, IsActiveContext cũng là một cái tên tệ. Context nào active? Nên là IdentiyActiveContext).
  • GetProfileData nhưng trả về void? Nó nên là LoadIdentityProfileDataContext. Nếu là như thế, không cần tham số context ở đây. Context là asyncScope, nên lấy nó từ store nào đó chứ không phải truyền vào. Luôn cố gắng tránh thay đổi tham số truyền vào. (giống như việc bạn truy cập HttpContext ở khắp nơi vậy. Với netframework là static property HttpContext.Current, với donet bạn inject IHttpContextAccessor).
  • Profile ở đây liệu đã rõ ràng? Nó có nên là IdentityProfileService? (Nếu không đọc concrete class, tôi đã nghĩ service này load user profile, tức các thông tin cá nhân ngoài các thông tin cơ bản).

Lại một service khác cùng thư mục: https://github.com/dotnet/eShop/blob/main/src/Identity.API/Services/ILoginService.cs

namespace eShop.Identity.API.Services
{
    public interface ILoginService<T>
    {
        Task<bool> ValidateCredentials(T user, string password);

        Task<T> FindByUsername(string user);

        Task SignIn(T user);

        Task SignInAsync(T user, AuthenticationProperties properties, string authenticationMethod = null);
    }
}

Các câu hỏi đặt ra:

  • FindByUsername có nên xuất hiện ở đây? LoginService là lo việc Login hay là một service tiện ích?
  • SignIn và SignInAsync có tham số khác nhau quá nhiều. Liệu dùng Sync và Async có khác nhau?
  • ValidateCredentials để làm gì? Vậy khi gọi SignIn sẽ validate hay không validate credential?

Nếu các bạn mò mẫm thì có thể tìm thấy nhiều những chỗ mắc các lỗi tương tự. Đúng vậy, không chỉ mình bạn, các lập trình viên tại Microsoft, trong một team viết framework cho cả thế giới dùng, cũng sẽ mắc các lỗi như thế. Các interface được thiết kế khiến chúng ta dễ hiểu nhầm về trách nhiệm của chúng.

Các bạn có để ý cả hai ví dụ trên tôi đều lấy từ các service class? Chính là thế. Các service class là các ổ đa nhiệm trong mã nguồn. Càng về lâu về dài, các service class càng dễ biến thành các lớp tiện ích, làm đủ việc. Hãy nhìn vào mã nguồn các dự án của bạn, chả khó gì tìm ra các service class dài thườn thượt, gồm các hàm liên kết với nhau chỉ bởi có chung prefix với class. UserService sẽ làm mọi thứ có chữ User. OrderService sẽ làm mọi thứ có chữ Order. Cơ sở này thường thì đúng khi mới tạo class, nhưng cũng là cách làm chúng ta trở nên dễ dãi và biến class đó thành một con quái vật sau này. Cái này cũng là điểm yếu của kiến trúc n-tier, được sửa chữa trong clean architecture, vì sao mà trên lớp entity trong clean architecture lại được gọi là use-cases chứ không phải là business logic layer, application service. Chính là tập trung vào trách nhiệm, vai trò của mã nguồn được tạo ra. OrderService nên được thay thế bằng PlaceOrderCommand, hay OrderModifier, hay CancelOrderTask, tức là các class có vai trò chuyên biệt, rõ ràng, gắn với use-case. Như thế bạn sẽ giữ được sự trong sáng, súc tích cho các class ấy. Không có lý do gì khi phát sinh yêu cầu RefundOrder bạn lại sửa đổi PlaceOrder hay CancelOrder. Đó là các use-case riêng biệt. Đóng gói theo các use-case phản ánh hành vi của hệ thống tốt hơn so với đóng gói theo n-tier hay onion (bám theo chức năng kĩ thuật của mã hơn là lý do mã đó được sinh ra - giải quyết một yêu cầu từ thực tế). Với OrderService sẽ chứa tất cả các thứ liên quan đến order, và sẽ đến lúc bạn đứng giữa ngã ba đường, nên đặt hàm này vào OrderService hay CustomerService (vd hàm CountOrders nên để trong đâu trong hai service trên?)?

Ok. Vậy chúng ta thông cảm cho nhau những lý do về mặt kiến trúc cũng như quy ước. Bài sau sẽ phân tích một số lý do đến từ kĩ năng, kỉ luật của lập trình viên.


All rights reserved

Viblo
Hãy đăng ký một tài khoản Viblo để nhận được nhiều bài viết thú vị hơn.
Đăng kí