بررسی Bad code smell ها: الگوی Shotgun Surgery
اندازه‌ی قلم متن
تخمین مدت زمان مطالعه‌ی مطلب: چهار دقیقه

برای مشاهده طبقه بندی Bad code smell‌ها می‌توانید به اینجا مراجعه کنید.
زمانیکه به ازای هر تغییر، نیاز باشد تغییرات کوچکی در تعداد کلاس‌های زیادی انجام شود، این بوی بد کد بوجود آمده است. این الگو از دسته بندی «جلوگیری کنندگان از تغییر» است. نام این دسته بندی به طور واضح گویای مشکلی است که این الگوی بد ایجاد می‌کند. 

چرا چنین بویی به راه می‌افتد؟ 

یکی از نشانه‌های وجود چنین الگوی بدی در کدها، مشاهده کدهای تکراریست. ریشه اصلی این بوی بد، پراکنده کردن مسئولیت‌ها در کلاس‌های مختلف است. مسئولیت‌هایی که بهتر بود در یک کلاس جمع شوند. معمولا برای رفع این بوی بد اقدام به جمع کردن مسئولیت‌ها از نقاط مختلف به یک کلاس می‌کنند.  
با توجه به توضیحات ارائه شده، این بوی بد عملا یکی از علایم اجرایی نکردن اصل Single responsibility  و Open closed از اصول طراحی شیء گرایی است. موارد دیگری که در ایجاد چنین مشکلی کمک می‌کنند به صورت زیر هستند:
  • استفاده نادرست از الگوهای طراحی شیء گرا 
  • عدم درک درست مسئولیت‌های کلاس‌های ایجاد شده 
  • عدم تشخیص مکانیزم‌های مشترک در کد و جداسازی مناسب آنها 
برای بررسی بیشتر این موضوع فرض کنید کلاس‌هایی در نرم افزار خود دارید که شماره تلفن کاربر را به صورت ورودی دریافت و روی آن کار خاصی را انجام می‌دهند. در ابتدای تولید نرم افزار فرمت صحیح شماره تلفن به صورت "04135419999" تشخیص داده شده است و مکانیزم اعتبارسنجی آن نیز با استفاده regular express‌ionها پیاده سازی شده‌است.  بعدا نیازمندی دیگری بوجود می‌آید که شماره تلفن‌هایی با کد بین المللی نیز در نرم افزار قابل استفاده باشند. مانند "984135410000+" دو نوع پیاده سازی (از میان روش‌های فراوان پیاده سازی) برای تشریح این موضوع می‌توان متصور بود. فرض کنید در دو موجودیت «کاربر» و «آدرس» نیاز به ذخیره سازی شماره تلفن وجود دارد. 

اول: هر جائیکه نیاز به اعتبارسنجی شماره تلفن وجود داشته باشد؛ این کار تماما در همان مکان انجام شود.
public class UserService 
{ 
        public void SaveUser(dynamic userEntity) { 
            var regEx = "blablabla"; 
            var phoneIsValid = Regex.IsMatch(userEntity.PhoneNumber, regEx); 
            if (!phoneIsValid) 
                return; 
            // ... 
        } 
}  

public class AddressService 
{ 
        public void SaveAddress(dynamic addressEntity) 
        { 
            var regEx = "blablabla"; 
            var phoneIsValid = Regex.IsMatch(addressEntity.PhoneNumber, regEx); 
            if (!phoneIsValid) 
                return; 
        } 
}
در این روش پیاده سازی اگر دقت کرده باشید روال مربوط به اعتبارسنجی در دو متد «ذخیره کاربر» و «ذخیره آدرس» تکرار شده‌است . این الگوی کد نویسی، علاوه بر این که خود نوعی بوی بد کد محسوب می‌شود، باعث ایجاد الگوی Shotgun surgery نیز است.  
در اینجا اگر قصد اعمال تغییری در منطق مربوط به اعتبارسنجی شماره تلفن وجود داشته باشد، نیاز خواهد بود تمامی مکان‌هایی که این منطق پیاده سازی شده‌است، بسته به شرایط جدید تغییر کند. یعنی برای تغییر یک منطق اعتبارسنجی نیاز خواهد بود کلاس‌های زیادی تغییر کنند.  

دوم: راه بهتر در انجام چنین کاری، جداسازی منطق مربوط به اعتبارسنجی شماره تلفن و انتقال آن به کلاسی جداگانه‌است؛ به صورت زیر: 
public class PhoneValidator
{ 
        public bool IsValid(string phoneNumber) 
        { 
            var regEx = "blablabla"; 
            var phoneIsValid = Regex.IsMatch(phoneNumber, regEx); 
            if (!phoneIsValid) 
                return false; 
            return true; 
        } 
 } 
 
public class UserService 
{ 
        public void SaveUser(dynamic userEntity) 
        { 
            var validator = new PhoneValidator(); 
            var phoneIsValid  = validator.IsValid(userEntity.PhoneNumber); 
            if (!phoneIsValid) 
                return; 
            // ... 
        } 
 } 
 
public class AddressService 
{ 
        public void SaveAddress(dynamic addressEntity) 
        { 
            var validator = new PhoneValidator(); 
            var phoneIsValid = validator.IsValid(addressEntity.PhoneNumber); 
            if (!phoneIsValid) 
                return; 
           // ... 
        } 
}

اگر به تکه کد بالا دقت کنید، مشاهده خواهید کرد که برای اعمال تغییر در منطق اعتبارسنجی شماره تلفن دیگر نیازی نیست به کلاس‌های استفاده کننده از آن مراجعه کرد و اعمال تغییر در یک نقطه کد، بر تمامی استفاده کنندگان اثر خواهد گذاشت. یکی دیگر از مزیت‌های استفاده از چنین روش پیاده سازی ای، امکان تست نویسی بهتر برای واحدهای مختلف کد است. 

شکل دیگر 

شکل دیگر این بوی بد کد، Divergent Change است. با این تفاوت که در الگوی Divergent Change تغییرات در یک کلاس اتفاق می‌افتند نه در چندین کلاس به طور همزمان. 

جمع بندی

تشخیص چنین الگوی بد کد نویسی ای همیشه به این سادگی نیست. یکی از راه‌های تشخیص سریع چنین بوی بد کدی این است که به کارهای تکراری عادت نکنید! و زمانیکه متوجه شدید کار خاصی را در کد به صورت تکراری انجام می‌دهید، دقت لازم را برای تغییر آن داشته باشید؛ به صورتیکه نیاز به اعمال تغییرات تکراری در مکان‌های مختلف کد وجود نداشته باشد. راه دیگر زمانی است که کدی تکراری را مشاهده کردید. زمانیکه کدی تکراری در کدها وجود داشته باشد، اطمینان داشته باشید هنگام تغییر آن به این مشکل دچار خواهید شد. برای رفع موضوع کد تکراری می‌توانید از روش‌های مختلفی که عنوان شد استفاده کنید. 
  • #
    ‫۷ سال و ۳ ماه قبل، دوشنبه ۲۹ خرداد ۱۳۹۶، ساعت ۱۴:۰۵
    به نظر حقیر بهتر بود کلاس PhoneValidator  به صورت استاتیک تعریف میشد چرا که داده ایی نداره و از طرفی نمونه سازی از اون تو کلاس‌های دیگه بدون تزریق (مانند همین جا در کلاس UserService  )، یکی از اصول شی گرایی رو نقض میکنه. لطفا اگه اشتباه فکر میکنم اساتید راهنمایی بفرمایید.
    • #
      ‫۷ سال و ۳ ماه قبل، دوشنبه ۲۹ خرداد ۱۳۹۶، ساعت ۱۹:۳۳
      یعنی new جزو اصول شیءگرایی نیست؟ پس چرا داخل یک زبان شیءگرا قرارگرفته؟ هدف از تزریق وابستگی‌ها چی هست؟ چه چیزی باید تزریق بشه؟ معکوس سازی وابستگی‌ها چی هست؟ آیا درون یک لایه هم باید وابستگی‌ها معکوس بشن یا این معکوس سازی مربوط به لایه‌های مختلف هست (* و *)؟ آیا زمانیکه شخصی سعی در آموزش مطلب ویژه و خاصی داره باید بیاد نویز جانبی ایجاد کنه (کلاس استاتیک، تزریق وابستگی، معکوس سازی وابستگی‌ها، استخراج اینترفیس، چند ریختی، داشتن چندین و چند اعتبارسنج، اصلا طراحی یک سیستم اعتبارسنجی کامل این وسط) یا اینکه تمرکزش رو بذاره روی اصل مطلبی که می‌خواد توضیح بده و در چهارچوب اون کار کنه؟
      • #
        ‫۷ سال و ۳ ماه قبل، دوشنبه ۲۹ خرداد ۱۳۹۶، ساعت ۱۹:۵۳
        دوست عزیز بحث بنده مفهومی و انتزاعی بود و نه مصداقی در مورد این مطلب. سوالی برام پیش اومده بود که خواستم دوستان اگه میتونن بنده رو راهنمایی بفرمایند.
        • #
          ‫۷ سال و ۳ ماه قبل، دوشنبه ۲۹ خرداد ۱۳۹۶، ساعت ۲۲:۲۹
          یکی از دلایلی که این کلاس رو بهتره به صورت استاتیک در نیاریم اینه که تست نوشتن برای کلاس در صورتی که کلاس validator استاتیک می‌بود سخت میشد. زیرا شاید در شرایطی نیاز می‌بود که این کلاس mock شود تا بتوان موارد دیگری از کلاس را تست کرد. 
          البته برای این که نیاز نباشد همیشه کلاس‌های این چنینی را به کلاس پاس داد یک الگوی بینابینی در دات نت وجود دارد به اسم ambient context که هم تا حدودی تست پذیر است و نیازی به ارسال پارامتر نیست.
          https://blogs.msdn.microsoft.com/ploeh/2007/07/23/ambient-context/ 
    • #
      ‫۷ سال و ۳ ماه قبل، دوشنبه ۲۹ خرداد ۱۳۹۶، ساعت ۲۰:۴۶
      جناب حسینی مثال ذکر شده برای توضیح موضوع مربوط به مطاب آماده شده. در نوشته هم ذکر کردم که این روش پیاده سازی یکی از چندین روش پیاده سازی برای این موضوع هست. سوالی که داشتید موضوع قابل بحث خوبی است ولی به موضوع این مطلب ارتباطی نداره. در صورت نیاز در این مورد میشه مفصل در جای دیگری صحبت بشه.
      • #
        ‫۷ سال و ۳ ماه قبل، شنبه ۳ تیر ۱۳۹۶، ساعت ۱۳:۱۵
        دوست عزیز ضمن تشکر از مطلب مفیدتون.بنظرت اون قسمت new PhoneValidator()  کد تکراری به حساب نمیاد و خودش باعث shotgun surgery  نمیشه؟بهتر نیست یک IPhoneValidator داشته باشیم و به کلاس سرویس تزریق کنیم تا در صورتی که تغییری در  PhoneValidator  بوجود اومد کد ما دستخوش تغییر نشه؟
        • #
          ‫۷ سال و ۳ ماه قبل، شنبه ۳ تیر ۱۳۹۶، ساعت ۱۳:۲۸
          به این مفهوم open closed principle می‌گن یا الگوی استراتژی.
          • #
            ‫۷ سال و ۳ ماه قبل، شنبه ۳ تیر ۱۳۹۶، ساعت ۱۸:۱۹
            طبق توضیحات گفته شده در اول مقاله 
            «زمانیکه به ازای هر تغییر، نیاز باشد تغییرات کوچکی در تعداد کلاس‌های زیادی انجام شود، این بوی بد کد بوجود آمده است. این الگو از دسته بندی «جلوگیری کنندگان از تغییر» است. نام این دسته بندی به طور واضح گویای مشکلی است که این الگوی بد ایجاد می‌کند.»    
            من فکر میکنم اون مسئله‌ای که گفتم شامل همین مباحث هستش و تغییر در PhoneValidator ممکنه تغییرات کوچک زیادی رو شامل بشه.
        • #
          ‫۷ سال و ۳ ماه قبل، شنبه ۳ تیر ۱۳۹۶، ساعت ۱۳:۴۶
          درست می‌فرمایید. پیاده سازی ای که فرمودید بهتر هست. و ساخته شدن شی در کلاس‌ها خودش به این کد بد بو ختم خواهد شد. و البته این روزها با گسترش استفاده از تکنیک‌های inversion of control و dependency injection خیلی از ما اتوماتیک پیاده سازی پیشنهادی شما رو انجام میدیم. 
          ولی یکی از چالش هایی که برای توضیح مطالب این چنین وجود داره اماده کردن مثال هست. معمولا سعی من بر اینه که مثال‌ها به صورتی باشه که فقط مفهوم ذکر شده در مطلب رو پوشش بده و درکش خیلی ساده باشه. طبیعتا در این مثالها موارد اشکال از زاویه‌های دیگه وجود خواهد داشت و من هم سعی میکنم این موضوع رو ذکر کنم که مثال‌ها فقط در جهت توضیح مطلب هست و پیاده سازی نهایی نیست. در بعضی موارد فراموش می‌کنم این نکته رو ذکر کنم. 
          به طور مثال در این مطلب سعی من توضیح موضوع shotgun surgery بود به سادترین روش ممکن. اگر موضوع dependency injection رو هم مطرح میکردم شاید مطالعه موضوع برای دوستانی که هنوز این مطلب براشون جا نیافتده مشکل‌تر میشد. 
          امیدوارم پاسخ سوالتون رو داده باشم.