From 8055d4ec5d95a15ec1c643c4e5d9c007a104cfb5 Mon Sep 17 00:00:00 2001 From: Yoga Pangestu Date: Thu, 3 Sep 2026 12:52:32 +0700 Subject: [PATCH] refactor: optimize AcademicTermService methods and improve query handling --- BEST_PRACTICE.md | 30 +++++++---- .../Admin/Master/AcademicTermController.php | 2 +- .../Admin/Master/AcademicTermService.php | 54 +++++++++---------- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/BEST_PRACTICE.md b/BEST_PRACTICE.md index 2e95424..cbe387a 100644 --- a/BEST_PRACTICE.md +++ b/BEST_PRACTICE.md @@ -241,18 +241,26 @@ ## 5. Kode Ringkas & Idiomatis Laravel | `App::make('Class')` | `app('Class')` | - **Jangan panggil `env()` di luar file config.** Simpan dulu ke `config/*.php`, lalu akses lewat `config('nama.key')`. Ini berlaku juga untuk kredensial/flag baru yang ditambahkan. -- **Tidak pakai DocBlock** untuk mendeskripsikan fungsi. Gunakan nama method yang deskriptif + return type/param type hint dari PHP. DocBlock hanya boleh dipakai kalau memang dibutuhkan untuk generic type (`@return Collection`) yang tidak bisa diekspresikan native PHP — bukan untuk narasi. - ```php - // Bad - /** - * The function checks if given string is a valid ASCII string - * @param string $string - * @return bool - */ - public function checkString($string) { } +- **DocBlock boleh dipakai kalau memang dibutuhkan dan penting** — bukan default yang dipasang di semua method. Untuk method standar (CRUD biasa, atau apa pun yang sudah jelas maksudnya dari nama method + return/param type PHP), **jangan** tambah DocBlock — itu pemborosan. DocBlock baru layak ditulis untuk kasus yang memang penting, misalnya: + 1. **Generic type** yang tidak bisa diekspresikan native PHP, mis. `@return Collection`, `@param array`. + 2. **Perilaku non-obvious** yang bisa bikin pembaca salah paham kalau tidak dijelaskan: aturan bisnis tersembunyi, constraint yang tidak kelihatan dari nama method, workaround keterbatasan interface/library. - // Good - public function isValidAsciiString(string $string): bool { } + ```php + // Bad — DocBlock generik yang cuma mengulang nama method, tidak nambah informasi + /** + * Get the validation rules that apply to the request. + */ + public function rules(): array { ... } + + // Good — tanpa DocBlock sama sekali, karena nama + return type sudah cukup jelas + public function rules(): array { ... } + + // Good — DocBlock dipertahankan karena memang menjelaskan hal yang tidak + // kelihatan dari signature method (fallback logic yang bisa mengejutkan) + /** + * Falls back to the one with the latest start date if none is explicitly active. + */ + public function getActive(): ?AcademicTerm { ... } ``` - Komentar kode hanya untuk menjelaskan **kenapa**, bukan **apa** — kalau nama variabel/method sudah jelas, jangan tambah komentar. diff --git a/app/Http/Controllers/Admin/Master/AcademicTermController.php b/app/Http/Controllers/Admin/Master/AcademicTermController.php index eee8a25..c363a39 100644 --- a/app/Http/Controllers/Admin/Master/AcademicTermController.php +++ b/app/Http/Controllers/Admin/Master/AcademicTermController.php @@ -24,7 +24,7 @@ public function index(PaginatedRequest $request): Response 'academicTerms' => $this->service->paginated( ...$request->validatedWithDefaults(), semester: $request->validated('semester'), - isActive: $request->filled('is_active') ? filter_var($request->validated('is_active'), FILTER_VALIDATE_BOOLEAN) : null, + isActive: $request->filled('is_active') ? $request->boolean('is_active') : null, ), 'filters' => $request->only(['semester', 'is_active']), ]); diff --git a/app/Services/Admin/Master/AcademicTermService.php b/app/Services/Admin/Master/AcademicTermService.php index e70c876..50ba89e 100644 --- a/app/Services/Admin/Master/AcademicTermService.php +++ b/app/Services/Admin/Master/AcademicTermService.php @@ -9,30 +9,14 @@ class AcademicTermService { - public function getAllForSelect(): Collection - { - return AcademicTerm::select(['id', 'academic_year', 'semester', 'start_date', 'end_date', 'is_active', 'open_semesters'])->latest()->get(); - } - - /** - * The academic term currently marked active, falling back to the one with - * the latest start date if none is explicitly active. - */ - public function getActive(): ?AcademicTerm - { - return AcademicTerm::where('is_active', true)->first() - ?? AcademicTerm::orderByDesc('start_date')->first(); - } - public function paginated(int $perPage = 25, string $search = '', ?string $semester = null, ?bool $isActive = null): LengthAwarePaginator { return AcademicTerm::query() - ->select(['id', 'academic_year', 'semester', 'start_date', 'end_date', 'is_active', 'open_semesters']) - ->when($search, fn($q) => $q->where('academic_year', 'like', "%{$search}%")) - ->when($semester, fn($q) => $q->where('semester', $semester)) - ->when($isActive !== null, fn($q) => $q->where('is_active', $isActive)) + ->when($search, fn ($q) => $q->where('academic_year', 'like', "%{$search}%")) + ->when($semester, fn ($q) => $q->where('semester', $semester)) + ->when($isActive !== null, fn ($q) => $q->where('is_active', $isActive)) ->latest() - ->paginate($perPage); + ->paginate($perPage, ['id', 'academic_year', 'semester', 'start_date', 'end_date', 'is_active', 'open_semesters']); } public function create(array $data): AcademicTerm @@ -45,13 +29,14 @@ public function create(array $data): AcademicTerm public function update(AcademicTerm $academicTerm, array $data): AcademicTerm { - $academicTerm->academic_year = $data['academic_year']; - $academicTerm->semester = $data['semester']; - $academicTerm->start_date = $data['start_date']; - $academicTerm->end_date = $data['end_date']; - $academicTerm->is_active = $data['is_active']; - $academicTerm->open_semesters = $this->normalizeOpenSemesters($data['open_semesters'] ?? []); - $academicTerm->update(); + $academicTerm->update([ + 'academic_year' => $data['academic_year'], + 'semester' => $data['semester'], + 'start_date' => $data['start_date'], + 'end_date' => $data['end_date'], + 'is_active' => $data['is_active'], + 'open_semesters' => $this->normalizeOpenSemesters($data['open_semesters'] ?? []), + ]); return $academicTerm; } @@ -72,6 +57,21 @@ public function updateActiveStatus(AcademicTerm $academicTerm, bool $isActive): }); } + public function getAllForSelect(): Collection + { + return AcademicTerm::latest()->get(['id', 'academic_year', 'semester', 'start_date', 'end_date', 'is_active', 'open_semesters']); + } + + /** + * The academic term currently marked active, falling back to the one with + * the latest start date if none is explicitly active. + */ + public function getActive(): ?AcademicTerm + { + return AcademicTerm::where('is_active', true)->first() + ?? AcademicTerm::orderByDesc('start_date')->first(); + } + /** * @param array $openSemesters * @return array