Comprehensive Code Review

Assessment Breakdown

No Database Foreign Key Constraints

What You Did

// Migration
$table->foreignId('kartupasien_id'); // Just creates integer column
// NO ->constrained() or ->references()

Why This Is FATAL

• Data orphans WILL happen when concurrent deletes occur
• No database-level protection against referential integrity violations
• Race conditions: User A deletes kartupasien while User B adds diagnosis
• Database cannot enforce relationships - relies 100% on application code
• Backup/restore nightmares - orphaned records in production dumps

Real-World Scenario

What Interviewers Will Think

How to Fix

// Migration - ADD THIS
Schema::create('vitalitas', function (Blueprint $table) {
    $table->id();
    $table->foreignId('odontogram_id')
          ->constrained('odontograms')
          ->onDelete('cascade'); // Or 'restrict' for safety
    // ... rest
});

No CSRF Protection on AJAX Requests

What You Did

// View code - Missing CSRF token in AJAX
$.ajax({
    url: '/getPenyebabs',
    type: 'POST',
    data: { askepgilut: askepgilut }, // NO _token!
    cache: false,
    success: function(msg) { ... }
});

But I Saw This

$.ajaxSetup({
    headers: {
        'X-CSRF-TOKEN': $('meta[name="csrf-token"]').attr('content')
    }
});

The Problem

• This $.ajaxSetup() is per-page setup
• If forgotten on ONE page → security hole
• Laravel will reject without token (unless you excluded routes in VerifyCsrfToken.php)
• Did you exclude these routes? If yes → HUGE security vulnerability

What Interviewers Will Ask

How to Fix (Proper Way)

// ALWAYS include token per request (safer than global setup)
$.ajax({
    url: '/getPenyebabs',
    type: 'POST',
    data: {
        _token: $('meta[name="csrf-token"]').attr('content'),
        askepgilut: askepgilut
    },
    success: function(msg) { ... }
});

String-Based Foreign Keys (pembimbing)

What You Did

// Migration
$table->string('pembimbing'); // Stores NIM as string!!!

Why This Is BAD

• Not a real FK - can't join efficiently
• No referential integrity - pembimbing can be deleted, string remains
• String comparison slower than integer joins
• Violates normalization - user data duplicated

Example of Problem

-- This query is SLOW (string comparison)
SELECT * FROM diagnosas 
WHERE pembimbing = '202301234';

-- vs proper FK (integer comparison + indexed)
SELECT * FROM diagnosas 
WHERE pembimbing_id = 15;

Proper Design

// Migration
$table->foreignId('pembimbing_id')
      ->constrained('users')
      ->onDelete('restrict'); // Prevent deleting active supervisor

// Model
public function pembimbing() {
    return $this->belongsTo(User::class, 'pembimbing_id');
}

Delimited String Storage (Violates 1NF)

What You Did

// diagnosas table
'askepgilut' => '1|2|3',
'penyebab'   => '1,2|3,4|5,6',
'gejala'     => '7,8|9,10|11,12'

Why This Is BAD

• Violates First Normal Form - atomic values rule
• Cannot query efficiently - must use LIKE '%1%' (no indexes)
• Cannot enforce FK constraints - DB doesn't know '1' is askepgilut.id
• Error-prone parsing - explode('|') logic fragile
• Cannot aggregate - "How many diagnoses use askepgilut #3?" → complex

Interviewer Will Ask

Your Answer Would Be

// Horrible - full table scan
$diagnosas = Diagnosa::where('penyebab', 'LIKE', '%5%')->get();
// Also matches '15', '25', '51' → WRONG RESULTS!

Proper Design

// Create pivot tables
Schema::create('diagnosa_askepgilut', function($table) {
    $table->foreignId('diagnosa_id')->constrained()->onDelete('cascade');
    $table->foreignId('askepgilut_id')->constrained()->onDelete('cascade');
    $table->primary(['diagnosa_id', 'askepgilut_id']);
});

// Model
class Diagnosa extends Model {
    public function askepgiluts() {
        return $this->belongsToMany(Askepgilut::class);
    }
}

// Clean queries
$diagnosa->askepgiluts; // Eloquent handles it
Askepgilut::find(5)->diagnosas; // Reverse query works

Helper Functions in Global Namespace

What You Did

// helper.php
if (!function_exists('getPatients')) {
    function getPatients($user_id) {
        // Returns HTML string
        return '<option>...</option>';
    }
}

Why This Is BAD

• Pollutes global namespace - name collision risk
• Not testable - how do you unit test these?
• Mixes concerns - business logic (query) + presentation (HTML)
• Not reusable - tightly coupled to specific HTML structure

What Senior Devs Expect

// app/Services/PatientService.php
namespace App\Services;

class PatientService {
    public function getPatientsByUser(int $userId): Collection {
        return Kartupasien::where('user_id', $userId)->get();
    }
}

// app/View/Components/PatientSelect.php
class PatientSelect extends Component {
    public function __construct(public Collection $patients) {}
    
    public function render() {
        return view('components.patient-select');
    }
}

// Usage in Blade
<x-patient-select :patients="$patientService->getPatientsByUser($userId)" />

Benefits of Proper Approach

No Validation Classes (FormRequest)

What You Did

// Controller
public function store(Request $request) {
    // Inline validation or none at all
    $diagnosa = new Diagnosa();
    $diagnosa->askepgilut = $request->input('askepgilut');
    $diagnosa->save();
}

Why This Is BAD

• No centralized validation - rules scattered in controllers
• No reusability - can't reuse validation between store/update
• Controller bloat - validation logic mixed with business logic
• No custom messages - generic Laravel errors

Proper Pattern

// app/Http/Requests/StoreDiagnosaRequest.php
class StoreDiagnosaRequest extends FormRequest {
    public function authorize() {
        return Gate::allows('mahasiswa');
    }
    
    public function rules() {
        return [
            'diagnosa.*.askepgilut' => 'required|array|min:1',
            'diagnosa.*.askepgilut.*' => 'exists:askepgiluts,id',
            'diagnosa.*.penyebab' => 'required|array|min:1',
            'diagnosa.*.penyebab.*' => 'exists:penyebabs,id',
            'kartupasien_id' => 'required|exists:kartupasiens,id',
        ];
    }
    
    public function messages() {
        return [
            'diagnosa.*.askepgilut.required' => 'Pilih minimal 1 diagnosis',
        ];
    }
}

// Controller - CLEAN
public function store(StoreDiagnosaRequest $request) {
    // Request already validated, authorized
    $validated = $request->validated();
    // Business logic only
}

Manual Cascade Deletes (Race Condition Risk)

What You Did

public function destroy(Eksplakkal $eksplakkal) {
    Eksplakkal::destroy($eksplakkal->id);
    Periodontal::where('eksplakkal_id', $eksplakkal->id)->delete();
}

The Problem - Race Condition

Why This Happens

• No database-level FK = no lock on parent delete
• Two operations (delete parent, delete children) not atomic
• Window of vulnerability between operations

Proper Solution

// Migration - Let DB handle it
$table->foreignId('eksplakkal_id')
      ->constrained()
      ->onDelete('cascade'); // Database handles cascade atomically

// Controller - Simple & Safe
public function destroy(Eksplakkal $eksplakkal) {
    $eksplakkal->delete(); // DB cascades automatically
}

Alternative: Use Transactions

public function destroy(Eksplakkal $eksplakkal) {
    DB::transaction(function() use ($eksplakkal) {
        Periodontal::where('eksplakkal_id', $eksplakkal->id)->delete();
        $eksplakkal->delete();
    });
}

Summary of Moderate Priority Issues

1. No Service Layer (Severity: 6/10)

public function store(Request $req) {
    $data = Kartupasien::where('user_id', auth()->id())
        ->with(['diagnosas', 'odontogram'])
        ->first();
    
    // Complex business logic here
    $dmft = $this->calculateDMFT($data);
    $ohis = $this->calculateOHIS($data);
    
    return view('result', compact('data', 'dmft'));
}

// Controller
public function store(
    Request $req, 
    PatientService $service
) {
    $patient = $service->getPatientData(auth()->id());
    $assessment = $service->calculateAssessment($patient);
    
    return view('result', compact('patient', 'assessment'));
}

// Service
class PatientService {
    public function calculateAssessment($patient) {
        return [
            'dmft' => $this->calculateDMFT($patient),
            'ohis' => $this->calculateOHIS($patient),
        ];
    }
}

2. Inconsistent Naming (Severity: 5/10)

// Mixed conventions
$table->string('nama_pasien');        // snake_case
public function getPasiens() { }      // camelCase + Indonesian
class kartupasien extends Model { }   // lowercase + Indonesian

// Should be
$table->string('patient_name');       // snake_case English
public function getPatients() { }     // camelCase English
class PatientCard extends Model { }   // PascalCase English

3. No Repository Pattern (Severity: 5/10)

// Repository Pattern
interface PatientRepositoryInterface {
    public function findByUser(int $userId): ?Kartupasien;
    public function getAllWithDiagnosis(): Collection;
}

class EloquentPatientRepository implements PatientRepositoryInterface {
    public function findByUser(int $userId): ?Kartupasien {
        return Kartupasien::where('user_id', $userId)
            ->with(['diagnosas', 'odontogram'])
            ->first();
    }
}

4. Business Logic in Controller (Severity: 6/10)

// ❌ Controller doing calculations
public function show($id) {
    $data = Periodontal::find($id);
    
    // Complex ACC calculation in controller
    $acc = 0;
    if ($data->pl_kuadran_1 + $data->di_kuadran_1 > 0) {
        $acc += 1;
    }
    // ... more calculation logic
    
    return view('show', compact('data', 'acc'));
}

// ✅ Move to Model or Service
class Periodontal extends Model {
    public function calculateACC(): int {
        $acc = 0;
        if ($this->pl_kuadran_1 + $this->di_kuadran_1 > 0) {
            $acc += 1;
        }
        // ... calculation logic
        return $acc;
    }
}

// Controller becomes clean
public function show($id) {
    $data = Periodontal::find($id);
    return view('show', [
        'data' => $data,
        'acc' => $data->calculateACC()
    ]);
}

5. No Database Transactions (Severity: 6/10)

// ❌ Without transaction
public function store(Request $request) {
    $kartupasien = Kartupasien::create($data);
    Odontogram::create(['kartupasien_id' => $kartupasien->id]);
    Periodontal::create(['kartupasien_id' => $kartupasien->id]);
    // If third create fails, you have orphaned records!
}

// ✅ With transaction
public function store(Request $request) {
    DB::transaction(function() use ($request) {
        $kartupasien = Kartupasien::create($data);
        Odontogram::create(['kartupasien_id' => $kartupasien->id]);
        Periodontal::create(['kartupasien_id' => $kartupasien->id]);
        // All or nothing - data integrity preserved
    });
}

6. Potential N+1 Queries (Severity: 5/10)

7. Magic Numbers (Severity: 4/10)

// ❌ Magic numbers
if (auth()->user()->role == 1) { }
Gate::define('adminpembimbing', fn($user) => in_array($user->role, [1,2]));

// ✅ Use Enums (PHP 8.1+)
enum UserRole: int {
    case ADMIN = 1;
    case PEMBIMBING = 2;
    case MAHASISWA = 3;
    case PATIENT = 4;
    case KARYAWAN = 5;
}

// Migration
$table->tinyInteger('role')->default(UserRole::MAHASISWA->value);

// Model
protected $casts = ['role' => UserRole::class];

// Usage
if (auth()->user()->role === UserRole::ADMIN) { ... }

✅ Things You Did RIGHT

1. Protected Routes with Middleware

Route::resource('/user', UserController::class)->middleware('admin');

2. Used Eloquent Relationships

public function kartupasien() {
    return $this->belongsTo(Kartupasien::class);
}

3. Gates for Authorization

Gate::define('adminpembimbing', fn($user) => in_array($user->role, [1,2]));

4. Resource Controllers (RESTful)

Route::resource('/diagnosa', DiagnosaController::class);

5. Dependency Injection

public function store(Request $request) // Laravel injects Request

6. Client-Side Calculations

• ✅ Instant feedback for users
• ✅ Good UX with immediate validation
• ✅ Server still validates

7. Select2 Integration

• ✅ Searchable dropdowns
• ✅ Multi-select support
• ✅ Better UX for large datasets

8. Real-World Domain Knowledge

• ✅ DMF-T, OHI-S algorithms correct
• ✅ Clinical workflow understood
• ✅ Shows domain expertise

Impact on Code Quality Score

Current: 6.5/10 → After fixes: 8.5/10

Priority Matrix

This Weekend (Critical Fixes)

php artisan make:migration add_foreign_keys_to_all_tables

• Verify no routes in VerifyCsrfToken exceptions
• Add _token to all AJAX calls

DB::transaction(function() { ... });

This Month (Severe Fixes)

• Create migration
• Update queries
• Test thoroughly

• One per controller
• Move validation rules

• Rename models to PascalCase
• English method names

Next Month (Architecture Improvements)

When Asked About Weaknesses

When Asked About String-Delimited Storage

When Asked About Helpers

💼 Interviewer Red Flags

✨ Things That Will Impress

Your Code Level

What This Code Shows

Job Market Assessment

🎓 Learning Path (3 Months to Mid-Level)

📝 Conclusion

Practice Questions for Interviews:

• "Why didn't you use foreign key constraints?"
• "How would you prevent N+1 queries here?"
• "Walk me through how you'd refactor this diagnosis storage."
• "What happens if two users delete the same record simultaneously?"

Have answers ready. Not excuses—learning stories. 🚀