المفاهيم
هنفهم إيه؟ الهدف إن زميلك يقدر يعرف الكود بيعمل إيه ويغيره من غير ما يخمن أو يكسر سلوك قديم. هنستخدم أسماء واضحة، دوال بمستوى مفهوم، وفحوص تساعد refactoring يحافظ على النتيجة؛ جودة الكود هنا مرتبطة بتكلفة التغيير، مش بعدد السطور.
قبل ما تبدأ: محتاج تميّز بين method وclass، وتمرر قيمة لدالة أو تقرأ نتيجة. refactoring هو تغيير ترتيب الكود مع الحفاظ على سلوكه الظاهر. لو قواعد C# الأساسية مألوفة، الأمثلة هتوضح الباقي من خلال قبل/بعد.
المصطلحات اللي تحتها خط منقّط تقدر تضغط عليها لمعرفة معناها، وترجع تكمل من نفس مكانك.
#Naming و functions بمستوى واحدNaming and one level of abstraction
الكود بيتقري أكتر ما بيتكتب. اسم زي Process بيخلي القارئ يفتح التنفيذ عشان يفهمه؛ CalculateInvoiceTotal بيوضح النية. سمّي بالمعنى وبالوحدة، زي timeoutSeconds، واستخدم نفس مصطلحات البيزنس في كل الطبقات.
// Fragment inside an invoice service; collaborators are injected.
public async Task IssueAsync(Invoice invoice, CancellationToken ct)
{
Validate(invoice); // (1)
invoice.CalculateTotal(); // (2)
await repository.SaveAsync(invoice, ct); // (3)
}
- (1) التحقق خطوة باسم واضح؛ تفاصيل الحقول في method تانية.
- (2) الحساب مسؤولية الـ invoice، مش loop وسط تنسيق العملية.
- (3) الحفظ على نفس مستوى التجريد: خطوات use case، مش وتفاصيل business في نفس المكان.
صغّر الـ function حوالين مسؤولية مفهومة، مش عدد سطور ثابت. استخراج كل سطر في method لوحده ممكن يزود التنقل من غير فايدة. اسم Async و CancellationToken بيوضحوا عقد الاستدعاء.
#Guard clauses و fail fastGuard clauses and failing fast
الـ nested ifs بتخبّي المسار الطبيعي. ارفض الحالة غير الصالحة بدري، قبل أي side effect. لكن لو الرفض المتوقع جزء من البيزنس، رجّع نتيجة واضحة بدل ما تعتبره exception في البنية التحتية.
decimal Discount(Customer customer, decimal total)
{
if (total > 0)
{
if (customer.IsActive)
{
if (customer.IsPreferred) return total * 0.05m;
}
}
return 0;
}decimal Discount(Customer customer, decimal total)
{
if (total <= 0) return 0; // (1)
if (!customer.IsActive) return 0;
if (!customer.IsPreferred) return 0;
return total * PreferredDiscountRate; // (2)
}
const decimal PreferredDiscountRate = 0.05m;- (1) نفس الحالات اللي كانت بترجع صفر خرجت بدري؛ اختبر الحدود عشان ترتيب الشروط ما يغيّرش السلوك.
- (2) المسار الناجح واضح، والـ constant بتشرح معنى الرقم.
customer مش null. لو ده عقد public، استخدم ArgumentNullException.ThrowIfNull. Fail fast مش معناه توقّع كل الاحتمالات جوه كل method.#Magic numbers و comments مفيدةMagic numbers and useful comments
رقم من غير معنى بيصعّب تغيير القاعدة. استخرج constant لو المعنى ثابت، و configuration لو التشغيل بيغيّره. الـ comment المفيد بيشرح سبب غير واضح أو قيد خارجي؛ إعادة وصف السطر بتتقادم وبتعمل دوشة.
// Add 30 days.
var due = issuedAt.AddDays(30);const int InvoicePaymentTermDays = 30; // (1)
// Contract requires calendar days, including weekends.
var due = issuedAt.AddDays(InvoicePaymentTermDays); // (2)- (1) الاسم بيقول الـ 30 تخص إيه؛ لو المدة بتختلف حسب العقد، تبقى قيمة في العقد.
- (2) التعليق بيمنع حد يبدّل calendar days بـ business days بالغلط.
سيب الصفر والواحد لو معناهم واضح من السياق. ماتعملش constant باسم Thirty. احذف الكود المعلّق واعتمد على version control؛ وثّق القرار المعماري في لما يحتاج سياق أكبر.
#DRY و KISS و YAGNI من غير مبالغةDRY, KISS and YAGNI without overengineering
- مكانان يحسبان ضريبة نفس نوع الفاتورة بنفس قاعدة العمل. لو النسبة اتغيرت، تعديل مكان ونسيان الآخر يسبب تناقضًا؛ ده تكرار معرفة مناسب للاستخراج.
- أما دالتان متشابهتان شكليًا، إحداهما لضريبة والأخرى لخصم، فقد يتغيران لأسباب مختلفة. جمعهما في دالة عامة مع flags كثيرة قد يصعّب الفهم.
- يعالج تكرار المعرفة، يدفع لتصميم بسيط، و يمنع بناء احتياج متخيّل. استخرج المشترك عندما تتأكد من وحدة القاعدة، لا لمجرد تشابه السطور.
DRY يعني مصدر واحد لنفس المعرفة، مش تجميع أي سطور شبه بعض. KISS اختار أبسط تصميم بيحقق المتطلبات. YAGNI ماتعملش مرونة لمتطلبات لسه مش موجودة. دالتين شبه بعض ممكن يمثلوا قاعدتين بيتغيروا لأسباب مختلفة.
public interface IShippingFee
{
decimal Calculate(decimal subtotal);
}
public sealed class Checkout(IShippingFee shipping) // (1)
{
public decimal Total(decimal subtotal) =>
subtotal + shipping.Calculate(subtotal); // (2)
}
- (1) composition بتضيف سياسة شحن للـ checkout من غير شجرة وراثة.
- (2) الـ checkout بيطلب سلوك، ومش محتاج يعرف تفاصيل كل شركة شحن.
استخدم interface لما فيه variation حقيقي أو boundary محتاجة فصل. لو سياسة واحدة صغيرة ومفيش احتياج لفصل، method بسيطة كفاية. الوراثة تنفع لعلاقة substitutability حقيقية، لكن استخدامها لمجرد reuse بيعمل coupling مع الـ base class.
#Tell-don't-ask و Law of DemeterTell-don't-ask and the Law of Demeter
لما كل caller يسأل عن حالة الـ order ويقرر يعدّلها بنفسه، قواعد الانتقال بتتوزع. خلي الـ object يحمي قواعده. Law of Demeter بتقلل معرفة الكود بالتركيب الداخلي للـ collaborators؛ مش مجرد عدّ النقط في .
if (order.Status == OrderStatus.Pending)
order.Status = OrderStatus.Cancelled;
var postalCode = order.Customer.Address.PostalCode;order.Cancel(); // (1)
var destination = order.ShippingDestination; // (2)
// Inside Order:
public void Cancel()
{
if (Status != OrderStatus.Pending)
throw new InvalidOperationException("Order cannot be cancelled.");
Status = OrderStatus.Cancelled;
}- (1) الـ order بيحمي transition بنفسه؛ الـ Status يبقى private set.
- (2) عنوان الشحن snapshot له معنى في الطلب، بدل الوصول لعنوان العميل الحالي من كذا طبقة.
دي fragments لتوضيح التصميم، والـ result بدل exception ينفع للرفض المتوقع. مش لازم تحوّل لكائنات غنية بالسلوك، ولا تعمل forwarding methods لكل property. طبّق القاعدة لما تقلل coupling وتحمي business .
#كتالوج Code smells وطريقة علاجهاCode smells and their refactorings
الـ smell علامة إنك محتاج تفهم الكود، مش حكم إنه غلط. اختار refactoring حسب سبب التغيير، واختبر قبلها.
| Smell | العلامة والعلاج |
|---|---|
| Long method | خطوات كتير مختلطة: Extract Method بأسماء intention-revealing. |
| Large class | مسؤوليات مختلفة: Extract Class حسب cohesion. |
| Primitive obsession | string لأي حاجة: Value Object زي Money أو Email مع validation. |
| Feature envy | method مشغولة بداتا class تاني: Move Method عند صاحب الداتا. |
| Shotgun surgery | قاعدة واحدة تعديلها يلمس ملفات كتير: Move Method و Move Field لتجميع القاعدة. |
| Divergent change | class واحد بيتغير لأسباب مستقلة: Extract Class لفصل المسؤوليات. |
| Data clumps | نفس parameters بتتحرك مع بعض: Introduce Parameter Object. |
| Switch statements | نفس switch على type بيتكرر: Replace Conditional with Polymorphism أو strategy. |
| Speculative generality | hooks و abstractions محدش محتاجها: Inline Class أو Collapse Hierarchy وحذف الكود غير المستخدم. |
#Refactoring techniques قبل وبعدRefactoring techniques: before and after
الـ refactoring بيغيّر تركيب الكود ويحافظ على سلوكه المرئي. الأمثلة مستقلة ومختصرة؛ الـ types الخاصة بالبيزنس بتمثل collaborators موجودة في التطبيق.
var total = lines.Sum(x => x.Price * x.Quantity);
invoice.Total = total;invoice.Total = CalculateSubtotal(lines); // (1)
decimal CalculateSubtotal(IEnumerable<InvoiceLine> lines) =>
lines.Sum(x => x.Price * x.Quantity);SearchOrders(DateOnly from, DateOnly to, int page, int pageSize);public record OrderSearch(DateOnly From, DateOnly To, int Page, int PageSize);
// Method signature becomes SearchOrders(OrderSearch query). // (2)decimal Fee(string kind) => kind switch
{
"standard" => 20m,
"express" => 50m,
_ => throw new ArgumentException("Unknown shipping kind.")
};public interface IShipping { decimal Fee(); } // (3)
public sealed class StandardShipping : IShipping
{
public decimal Fee() => 20m;
}
public sealed class ExpressShipping : IShipping
{
public decimal Fee() => 50m;
}
// Resolve the validated kind at the boundary; do not silently use a default.public List<OrderLine> Lines { get; } = [];private readonly List<OrderLine> _lines = [];
public IReadOnlyList<OrderLine> Lines => _lines.AsReadOnly(); // (4)
public void AddLine(OrderLine line)
{
ArgumentNullException.ThrowIfNull(line);
_lines.Add(line);
}- (1) اسم الحساب فصل الـ how عن الـ what من غير تغيير المعادلة.
- (2) parameter object بيجمع قيم مرتبطة؛ زوّد التحقق من المدى والـ pagination في boundary مناسب.
- (3) كل shipping type بيملك سلوكه. تكلفة classes إضافية تستاهل لما التفرعات بتتكرر؛ الأرقام هنا أمثلة، والـ constant extraction اتوضح في كارت magic numbers.
- (4) read-only wrapper يمنع cast مباشر لـ List وتعديلها. ده مش بيجمّد عناصر الـ collection ولا بيعمل snapshot؛ لو مطلوب استخدم values ونسخة مستقلة.
#Refactoring آمن و technical debt و code reviewSafe refactoring, technical debt and code review
لو مفيش tests، ابدأ بـ characterization tests: اختبارات تسجل اللي البرنامج بيعمله حالياً، حتى الحالات الغريبة، عشان تعرف لو refactoring غيّر النتيجة. صلّح bug في تغيير منفصل عن refactoring عشان المراجع يميّز السلوك المقصود من إعادة ترتيب الكود.
- حدد المشكلة ومقياس التحسن: مكان قاعدة مكررة أو صعوبة إضافة حالة.
- ثبت السلوك بـ tests مناسبة، وراجع الـ public contracts (السلوك اللي callers بيعتمدوا عليه) والـ side effects زي حفظ الداتا أو إرسال رسالة.
- اعمل rename أو extraction صغيرة، شغّل tests، وراجع diff.
- راقب الأداء لو التغيير لمس hot path، وانشر تدريجياً لو فيه مخاطرة تشغيل.
Technical debt: حل سريع ليه تكلفة مستقبلية. سجل السبب، والأثر، والـ owner، وإشارة السداد؛ مش كل كود قديم محتاج rewrite.
Review checklist: صحة السلوك، و edge cases، و authorization، و data consistency، و cancellation، و logs من غير أسرار، و tests للسلوك، ووضوح الأسماء، وحجم الـ diff. افصل blocker عن اقتراح وعن nit.
#Analyzers و .editorconfig و nullableAnalyzers, .editorconfig and nullable warnings
اتفاقات مكتوبة بس بتستهلك وقت المراجعة. خلي الـ compiler والـ analyzers يلقطوا الأخطاء المتكررة. ده مثال إعدادات لمشروع 10؛ راجع القواعد تدريجياً في مشروع قديم بدل ما تعمل suppress لكل التحذيرات.
<!-- In the project file -->
<PropertyGroup>
<Nullable>enable</Nullable>
<AnalysisLevel>10.0</AnalysisLevel>
<WarningsAsErrors>$(WarningsAsErrors);nullable</WarningsAsErrors>
</PropertyGroup>
# .editorconfig
root = true
[*.cs]
indent_style = space
indent_size = 4
dotnet_diagnostic.CA2200.severity = warning
csharp_style_var_for_built_in_types = false:suggestion
Nullable بيشغّل null-state analysis، و WarningsAsErrors بيخلّي مجموعة nullable تفشل الـ build. CA2200 يحذر من rethrow بيغيّر stack information. الـ style suggestion مش ضمان إنها هتفشل ؛ فعّل severity و build enforcement المناسبين لو ده المطلوب.
| قديم | أنسب دلوقتي |
|---|---|
| Null checks عشوائية ومراجعة يدوية للتنسيق | Nullable analysis و analyzers و .editorconfig مشتركة |
| إسكات التحذير بـ ! من غير سبب | إصلاح عقد الـ null أو إثباته بـ validation |
الأدوات مش هتعرف لو شرط بيزنس ناقص. dotnet format --verify-no-changes مفيد لفحص التنسيق المتفق عليه في CI.
أسئلة الانترفيو
مفيش نتايج
راجع فهمك
ليه الاختيار ده مناسب؟ إمتى ممكن ماينفعش؟ وإيه اللي هيتغير لو الحمل زاد أو ظهر فشل؟
16.1What makes code clean?Junior
الكود الواضح بيوصل النية، وحدوده وقواعده مفهومة، وسهل تغييره بأمان. مش مسابقة أقل عدد سطور. اسم واضح ومسؤولية متماسكة واختبار للحدود أهم من pattern لمجرد الشكل.
I judge clean code by how safely another engineer can understand and change it. I look for meaningful names, cohesive functions, explicit dependencies and visible failure paths. I also want tests around business rules and boundaries, because readability alone does not establish correctness. I do not apply a fixed method length or introduce patterns automatically. A short function with hidden side effects can be harder to maintain than a longer, straightforward calculation.
16.2How do you choose good names?Junior
استخدم لغة الدومين، ووضح الوحدة والنتيجة، وخلّي الأفعال للسلوك. بلاش اختصارات محلية أو أسماء عامة زي manager لو مابتوضحش المسؤولية.
I choose names that express domain intent and make the call site readable. For example, paymentTimeoutSeconds is clearer than timeout, and ReserveStock is clearer than Process. I use the same term for the same concept throughout the application and distinguish different concepts even when their current implementation matches. I follow C# conventions such as the Async suffix and avoid abbreviations that require local knowledge. If naming is difficult, I check whether the responsibility itself is unclear.
16.3How small should a function be?Junior
قد المسؤولية، مش رقم معين. امزجش orchestration مع تفاصيل parsing وحسابات في نفس المستوى، لكن ماتستخرجش methods مالهاش معنى.
I keep a function focused on one coherent operation and roughly one level of abstraction. An orchestration method can validate an order, reserve inventory and save it, while detailed calculations live behind meaningful names. I do not use a universal line limit because a clear mapping or algorithm may need more lines. I extract a method when it introduces a useful concept or isolates complexity, and avoid splitting code so aggressively that reading it requires constant .
16.4Why use guard clauses?Junior
بتخرج الحالات الخاصة بدري وتسيب المسار الطبيعي واضح. حافظ على ترتيب الـ side effects وسلوك null والـ short-circuit أثناء التحويل.
I use guard clauses to handle invalid input and exceptional cases near the beginning of a method, leaving the normal path easy to follow. They reduce indentation and make preconditions visible. During refactoring, I preserve evaluation order, null handling and side effects, because flattening conditions can accidentally change behavior. I also distinguish invalid programming input from expected business rejection. The former may justify an argument exception, while the latter often deserves an explicit result.
16.5When should you write a comment?Junior
لسبب مش واضح، زي قيد عقد أو workaround له مرجع. لو التعليق بيشرح الكود عشان الاسم وحش، حسّن الاسم الأول.
I write comments for information the code cannot communicate well, such as a regulatory constraint, a deliberate trade-off or a workaround linked to an issue. I avoid comments that simply repeat the statement, and I rename or extract confusing code before explaining it in prose. Public documentation can describe contracts and failure behavior. I remove commented-out implementations because version control already preserves history, and I update useful comments whenever the relevant behavior changes.
16.6What does DRY actually mean?Mid
مصدر واحد لنفس القاعدة. التشابه الشكلي مش كفاية؛ ضريبة الفاتورة وخصم الولاء ممكن يتساووا النهارده ويتغيروا بكرة لأسباب مختلفة.
I interpret as avoiding multiple authoritative representations of the same knowledge. If the same tax rule is implemented in three places, a policy should own it. However, similar-looking code can represent independent rules, such as a loyalty discount and an invoice adjustment. Combining them can create coupling and complicated flags. I first ask whether the pieces must change together, then extract a shared concept when that relationship is real rather than deduplicating purely by appearance.
16.7How do KISS and YAGNI affect your design?Mid
يطلب بساطة تحقق المطلوب، و يمنع متطلبات متخيلة. البساطة مش معناها تجاهل الأمان أو الموثوقية المطلوبة.
I use to choose the simplest design that satisfies the actual functional and operational requirements. stops me from building extension points for scenarios nobody currently needs. For example, I would not introduce a plugin framework for one shipping calculation. I still preserve useful boundaries and meet security and reliability requirements, because those are real needs. When a second variation appears, I can refactor using evidence instead of carrying a speculative abstraction indefinitely.
16.8What is the difference between shotgun surgery and divergent change?Mid
Shotgun surgery: تغيير واحد يلمس أماكن كتير. Divergent change: مكان واحد بيتغير لأسباب كتير. الأول محتاج تجميع، والتاني غالباً فصل.
Shotgun surgery means one logical change requires coordinated edits across many files, often because a rule is scattered. Divergent change means one class changes for several unrelated reasons, often because it owns too many responsibilities. I address the first by moving related behavior and data together, and the second by extracting cohesive responsibilities. I verify the actual change history before restructuring, because class size or file count alone does not prove either problem.
16.9When would you replace a conditional with polymorphism?Mid
لما نفس التفرع على النوع يتكرر عبر عمليات، وكل نوع له سلوك متماسك. Switch صغير وواضح على حالات ثابتة ممكن يفضل أفضل.
I consider polymorphism when the same type-based branching appears in several operations and each variant has meaningful behavior. A shipping policy can then own its price and delivery rules, reducing coordinated switch changes. I keep construction or selection at a boundary and reject unsupported values explicitly. For one small switch over a stable enum, introducing a hierarchy may add more complexity than it removes. I choose based on repetition and change patterns rather than treating every switch as a defect.
16.10What are tell-don't-ask and the Law of Demeter?Mid
اطلب من الـ order يعمل cancel بدل ما تقرا state وتعدّلها من بره. وقلل الوصول للـ internals عبر سلاسل objects. القواعد مش حظر على أو .
Tell-don't-ask encourages me to request behavior from the object that owns a rule instead of reading its state and reproducing the rule elsewhere. The Law of Demeter limits dependence on a collaborator's internal object graph. For example, order.Cancel can protect valid transitions, and an order shipping snapshot avoids traversing customer internals. I apply these ideas to reduce coupling and protect , while keeping simple data transfer and query projections straightforward.
16.11Why prefer composition over inheritance?Mid
Composition بتخليك تبدّل سياسة مستقلة من غير الاعتماد على تفاصيل base class. الوراثة محتاجة علاقة is-a صحيحة والـ subtype يحافظ على العقد.
I prefer composition when behavior varies independently, such as shipping, discounts or payment providers. Injecting a collaborator lets me replace one policy without inheriting unrelated state or protected implementation details. It also makes dependencies visible and tests more focused. Inheritance remains useful when there is a genuine substitutable relationship and a stable base contract. I avoid deep hierarchies created only for code reuse, because base-class changes can unexpectedly affect every derived class.
16.12How do you refactor code that has no tests?Senior
ثبت السلوك المهم بـ characterization tests، واعمل seam لو dependencies تمنع الاختبار. خطوات صغيرة، وماتخلطش تغيير قواعد البيزنس بتحسين التركيب.
I first identify the behavior and contracts that must remain stable, including failure paths and side effects. I add characterization tests around those boundaries, using a small seam if necessary to isolate a dependency. Then I make one structural change at a time and run the relevant tests. I separate discovered bugs from refactoring so reviewers can see intentional behavior changes. For high-risk paths, I also compare production signals and use a gradual rollout with a clear rollback path.
16.13How do you prioritize technical debt?Senior
اربط الدين بتكلفة فعلية: incidents، أو وقت تعديل، أو مخاطرة. سجل السبب والـ owner وإمتى السداد يستاهل، بدل rewrite لمجرد إن الكود قديم.
I prioritize technical debt by its effect on delivery, reliability and risk rather than by how old or unattractive the code looks. I record the shortcut, its interest cost, the affected area and a practical repayment trigger. Repeated incidents or changes that consistently touch many files are useful evidence. I favor small improvements alongside related feature work, and propose a larger investment only when its expected benefit and migration risk justify the cost.
16.14How do you give effective code review feedback?Senior
اشرح السيناريو والضرر، وحدد blocker ولا suggestion. راجع الصحة والأمان الأول، وسيب التنسيق للأدوات. ناقش الكود مش الشخص.
I review the intended behavior first, then correctness, security, consistency and test coverage before style. My comments describe a concrete scenario and consequence, and suggest an action or ask a focused question. I label whether something blocks approval or is optional, and explain the trade-off when several solutions are reasonable. I leave mechanical formatting to tooling. If a discussion becomes lengthy, I resolve it directly and record the decision so the author has a clear next step.
16.15What can analyzers and nullable reference types enforce?Mid
بيلقطوا أنماط أخطاء و null-state ومخالفات متكررة. مش هيكتشفوا قاعدة business ناقصة، و ! مش runtime validation.
I use analyzers to catch repeatable defects and shared conventions, and nullable reference types to make nullability part of the compile-time contract. I can promote nullable warnings to build errors and share formatting or diagnostic severity through .editorconfig. This reduces review noise, but it does not prove the business logic is correct. The null-forgiving operator only suppresses analysis; it does not validate input at runtime. External input still needs validation, and behavior still needs appropriate tests.
اختبر نفسك
إجابات الاختبار وعلامات إكمال المحاور بتتحفظ على المتصفح ده، ومش بتتنقل لأجهزة تانية.
Two similar calculations change for unrelated business reasons. What is safest?
Which comment adds useful information?
One pricing change requires edits in twelve unrelated files. Which smell?
A class changes for shipping, billing and email requirements. Which smell?
What defines a refactoring?
What does a read-only collection wrapper guarantee?
What is the best first step before risky refactoring without tests?
What does the null-forgiving operator ! do?