From a52bb4e605d28f0a9778d405cef9521289ea098d Mon Sep 17 00:00:00 2001 From: mahdi Date: Wed, 18 Feb 2026 22:00:43 +0330 Subject: [PATCH] add code smell analysis and refactor plans for routing.py and utils.py --- rambo-refactors/session-01/Task.md | 47 ++++++++++++++ rambo-refactors/session-01/answer.md | 39 ++++++++++++ rambo-refactors/session-01/code_smells.md | 76 +++++++++++++++++++++++ 3 files changed, 162 insertions(+) create mode 100644 rambo-refactors/session-01/Task.md create mode 100644 rambo-refactors/session-01/answer.md create mode 100644 rambo-refactors/session-01/code_smells.md diff --git a/rambo-refactors/session-01/Task.md b/rambo-refactors/session-01/Task.md new file mode 100644 index 0000000..718c39a --- /dev/null +++ b/rambo-refactors/session-01/Task.md @@ -0,0 +1,47 @@ +# Rambo Refactors – Session 01 + +## TASK 1 – Refactor Practice + +**Goal:** Identify and refactor bad code patterns in the FastAPI API. + +### Target files + +- [`routing.py` – FastAPI](https://github.com/fastapi/fastapi/blob/f2687dc1bb01f4cb62eee90e656b61c38c2aaf6a/fastapi/routing.py#L4) +- [`utils.py` – FastAPI](https://github.com/fastapi/fastapi/blob/f2687dc1bb01f4cb62eee90e656b61c38c2aaf6a/fastapi/dependencies/utils.py#L4) + +### Instructions + +1. Identify code smells: + - Long functions + - Duplication + - Unclear variable names + - Implicit dependencies +2. Document your findings in `code_smells.md`. +3. Apply safe refactorings using small commits. +4. Each PR should be reviewable by team mates, focused on **one concrete refactor at a time**. + +### Output + +- `code_smells.md` with smell + refactor plan +- PRs with small, safe refactors + +--- + +## TASK 2 – Refactoring Checklist + +**Goal:** Create a checklist specifically for safe refactoring and technical debt reduction. + +### Instructions + +1. Review your findings from Task 1. +2. Create concrete rules for: + - Refactoring safely + - Maintaining tests + - Avoiding regressions + - Incremental improvements +3. Write in `refactor_checklist.md`. + +### Output + +- `refactor_checklist.md` +- Concrete examples of **before/after** code snippets diff --git a/rambo-refactors/session-01/answer.md b/rambo-refactors/session-01/answer.md new file mode 100644 index 0000000..37634a9 --- /dev/null +++ b/rambo-refactors/session-01/answer.md @@ -0,0 +1,39 @@ +# جمع‌بندی جلسه آنلاین — چهارشنبه ۲۹ بهمن + +--- + +## ما چی بررسی کردیم؟ + +دو تا فایل اصلی از سورس‌کد FastAPI رو بررسی کردیم: `routing.py` و `utils.py`. هدفمون پیدا کردن code smell ها و مشکلات ساختاری بود که باعث میشه نگهداری و توسعه کد سخت‌تر بشه. + +--- + +## به چه نتیجه‌ای رسیدیم؟ + +### فایل `routing.py` + +۱. **تکرار ۳,۰۰۰ خطی متدهای HTTP** — متدهای `get()`, `put()`, `post()`, `delete()`, `options()`, `head()`, `patch()`, `trace()` کاملاً کپی‌پیست هم‌دیگن. تنها تفاوتشون `methods=["GET"]` در مقابل `methods=["PUT"]` و ... هست. یعنی ۶۸ درصد فایل فقط تکراره. + +۲. **Shadow naming توی `request_response()`** — یه تابع داخلی دقیقاً هم‌نام تابع بیرونیش تعریف شده (هر دو `app` نام دارن) که گیج‌کننده‌ست. + +۳. **God Function: `get_request_handler()`** — حدود ۱۵۵ خط با ۶ مسئولیت مختلف: parse کردن body، تشخیص content-type، مدیریت خطا، حل dependency ها، اجرای endpoint، و ساختن response. + +### فایل `utils.py` + +۴. **متد `ensure_multipart_is_installed()` با سه لایه try/except تو در تو** — خوندنش سخته و چک version روی پکیج اشتباهی هم بی‌معنیه. + +۵. **متد `get_dependant()` بزرگ و چندوظیفه‌ای** — توی یه حلقه سه تا کار متفاوت انجام میده: هندل Depends، تشخیص تایپ‌های خاص مثل Request و WebSocket، و مسیریابی فیلدهای عادی. + +۶. **صدا زدن بازگشتی بدون مکانیزم قطع** — `get_dependant()` خودش رو recursive صدا میزنه ولی هیچ depth limit یا چک circular dependency نداره. همین مشکل برای `get_flat_dependant()` و `solve_dependencies()` هم وجود داره. ممکنه جای دیگه‌ای محافظتی باشه. + +۷. **Implicit Coupling بین `_should_embed_body_fields()` و `get_body_field()`** — هر دو روی `body_params` کار میکنن و خروجی اولی ورودی دومیه، ولی رابطه رسمی‌ای بینشون نیست. اگه کسی یکیشون رو بدون دیگری صدا بزنه باگ ساکت ایجاد میشه. + +--- + +## مهم‌ترین تصمیم‌ها و اختلاف‌نظرها + +- در مورد این که احتمالا برنامه نویس های FastApi باید افراد قوی از نظر فنی باشند پس چرا این مشکلات رو دارند بحث کردیم + +- ممکن است این فایل ها نسخه خاص در برنچ خاصی بوده باشه و بعدا خود تیم ریفکتور کرده باشند بحث کردیم + +- با کمک گیت و ابزار های گیت چطور میتونیم تغیرات رو متوجه بشیم و به code smell ها برسیم diff --git a/rambo-refactors/session-01/code_smells.md b/rambo-refactors/session-01/code_smells.md new file mode 100644 index 0000000..b948320 --- /dev/null +++ b/rambo-refactors/session-01/code_smells.md @@ -0,0 +1,76 @@ +# Code Smells — فایل‌های `routing.py` و `utils.py` + +--- + +## ۱. تکرار ۳,۰۰۰ خطی متدهای HTTP در `routing.py` + +**محل:** خط ۱۴۳۵ تا ۴۴۷۴ + +متدهای `get()`, `put()`, `post()`, `delete()`, `options()`, `head()`, `patch()`, `trace()` کاملاً کپی‌پیست هم‌دیگن. هر کدوم حدود ۳۷۵ خطن و تنها تفاوتشون یه خطه: + +```python +methods=["GET"] # توی get() +methods=["PUT"] # توی put() +methods=["POST"] # توی post() +# ... +``` + +از ۴,۵۰۰ خط کل فایل، حدود ۳,۰۰۰ خطش (۶۸٪) فقط تکراره. اگه پارامتر جدید اضافه بشه یا باگی فیکس بشه، باید ۸ جا عوض بشه. + +--- + +## ۲. God Function: `get_request_handler()` + +**محل:** `routing.py` خط ۲۴۸ تا ۴۰۳ (~۱۵۵ خط) + +یه تابع که ۶ تا مسئولیت مختلف داره: parse کردن body، تشخیص content-type، مدیریت خطای JSON، حل dependency ها، اجرای endpoint، و ساختن response. + +--- + +## ۳. try/except سه‌لایه تو در تو در `ensure_multipart_is_installed()` + +**محل:** `utils.py` خط ۸۵ تا ۱۰۹ + +سه لایه try/except تو در تو که دنبال کردن flow خطاهاش سخته. علاوه بر این وقتی پکیج اصلی نصب نیست، بیخودی version پکیج legacy رو چک میکنه . + +--- + +## ۴. متد `get_dependant()` بزرگ و چندوظیفه‌ای + +**محل:** `utils.py` خط ۲۵۱ تا ۳۲۳ + +توی یه حلقه `for` سه تا کار متفاوت انجام میده با الگوی if/continue: +- هندل کردن `Depends` (با validate scope و استخراج OAuth) +- تشخیص تایپ‌های خاص مثل `Request` و `WebSocket` +- مسیریابی فیلدهای عادی به body یا query/path/header/cookie + +--- + +## ۵. صدا زدن بازگشتی بدون مکانیزم قطع + +**محل:** `utils.py` — توابع `get_dependant()` خط ۲۹۸، `get_flat_dependant()` خط ۱۶۴، `solve_dependencies()` خط ۶۱۴ + +هر سه تابع خودشون رو recursive صدا میزنن تا nested dependency ها رو پردازش کنن ولی depth limit یا چک circular dependency مشخصی ندارن. اگه یه dependency به صورت دایره‌ای به خودش اشاره کنه، بدون پیام خطای واضح میره توی infinite recursion. + +نکته: `get_flat_dependant()` یه لیست `visited` داره ولی فقط وقتی `skip_repeats=True` باشه ازش استفاده میکنه. توی حالت پیش‌فرض عملاً بلااستفاده‌ست. + +با دقت بیشتر بررسی نکردیم — ممکنه جای دیگه‌ای محافظتی وجود داشته باشه. + +--- + +## ۶. توابع recursive بدون محدودیت + +**فایل:** `utils.py` + +سه تا تابع خودشون رو recursive صدا میزنن: +- `get_dependant()` خط ۲۹۸ خودش رو صدا میزنه +- `get_flat_dependant()` خط ۱۶۴ خودش رو صدا میزنه +- `solve_dependencies()` خط ۶۱۴ خودش رو صدا میزنه + +هیچ‌کدوم depth limit ندارن. یعنی اگه یه dependency دایره‌ای باشه و به خودش اشاره کنه، میره توی infinite recursion و stack overflow میده بدون اینکه خطای واضحی بده. + +`get_flat_dependant()` یه لیست `visited` داره ولی فقط وقتی `skip_repeats=True` باشه چکش میکنه. حالت پیش‌فرضش `False` هست یعنی عملاً بلااستفاده‌ست. + +البته با دقت بیشتر ندیدیم، شاید جای دیگه‌ای یه محافظتی باشه. + +--- \ No newline at end of file