diff --git a/.claude/guidelines.md b/.claude/guidelines.md index 5d115aa..d00cdc5 100644 --- a/.claude/guidelines.md +++ b/.claude/guidelines.md @@ -14,7 +14,7 @@ SimpleBatteryNotifier is an Android application that monitors battery status and - Records for simple data carriers - **Resource Management**: Use try-with-resources for automatic resource cleanup - **Null Safety**: Use `isNull()` and `nonNull()` from `java.util.Objects` -- **Immutability**: Use `final` for all method parameters and local variables where possible +- **Immutability**: Use `final` for local variables where possible. Do **not** put `final` on method/constructor parameters in new or edited code — effectively-final already covers lambda capture, so the keyword is just noise. Legacy code carries `final` params (an older convention) and migrates incrementally as methods are otherwise touched; a full sweep isn't required. - **Static Imports**: Import commonly used static methods (e.g., `isNull`, `nonNull`) ### Naming Conventions diff --git a/CODE_REVIEW_GUIDELINES.md b/CODE_REVIEW_GUIDELINES.md index 3a49668..2b46bd6 100644 --- a/CODE_REVIEW_GUIDELINES.md +++ b/CODE_REVIEW_GUIDELINES.md @@ -242,6 +242,19 @@ final int age = 25; **Why?** Improves readability and prevents accidental reassignment. +> **Parameters are the exception — do not mark method/constructor parameters `final`** in new or edited code. Effectively-final already covers lambda capture, so the keyword only adds noise. Legacy code still carries `final` params (an older convention) and migrates as methods are otherwise touched — a full sweep isn't required. +> +> ```java +> // ❌ BAD - final on parameters +> public int criticalLevel(final Context context) { ... } +> +> // ✅ GOOD - final on locals, not parameters +> public int criticalLevel(Context context) { +> final SharedPreferences prefs = getPrefs(context); +> // ... +> } +> ``` + ### 2. Declare Variables Close to Usage ```java @@ -278,7 +291,7 @@ public void process(String input) { } // ✅ GOOD - Early returns -public void process(final String input) { +public void process(String input) { if (isNull(input)) { return; } @@ -308,12 +321,12 @@ public void processUserDataAndSendEmail(User user) { } // ✅ GOOD - Separate concerns -public void processUser(final User user) { +public void processUser(User user) { validateUser(user); saveToDatabase(user); } -public void sendWelcomeEmail(final User user) { +public void sendWelcomeEmail(User user) { // Email logic only } ``` @@ -331,7 +344,7 @@ final NotificationManager manager2 = (NotificationManager) context.getSystemServ manager2.notify(id2, notification2); // ✅ GOOD - Extracted to method -private void sendNotification(final Context context, final int id, final Notification notification) { +private void sendNotification(Context context, int id, Notification notification) { final NotificationManager manager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); if (nonNull(manager)) { manager.notify(id, notification); @@ -371,7 +384,7 @@ public void unusedMethod() { notificationManager.notify(id, notification); // ✅ GOOD - Check permission first (Android 13+) -private boolean hasNotificationPermission(final Context context) { +private boolean hasNotificationPermission(Context context) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { return ContextCompat.checkSelfPermission(context, Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED; @@ -534,7 +547,7 @@ public void method2() { } // ✅ GOOD - Extract to helper -private NotificationManager getNotificationManager(final Context context) { +private NotificationManager getNotificationManager(Context context) { return (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); } ``` @@ -603,7 +616,7 @@ private boolean isInDoNotDisturbMode() { * @param context The application context * @param type Notification type (CRITICAL_TYPE, WARNING_TYPE, or FULL_LEVEL_TYPE) */ -public static void sendNotification(final Context context, final int type) { +public static void sendNotification(Context context, int type) { // Implementation } ``` @@ -646,7 +659,7 @@ When reviewing code, check: - [ ] Uses `isNull()`/`nonNull()` instead of `== null`/`!= null` - [ ] All curly brackets present (even single-line) -- [ ] Variables marked `final` where appropriate +- [ ] Locals marked `final` where appropriate; parameters **not** marked `final` - [ ] No FQNs (uses imports) - [ ] Line width ≤ 160 characters - [ ] Early returns instead of deep nesting diff --git a/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java index a5630d8..8406a6c 100644 --- a/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java +++ b/app/src/main/java/com/almothafar/simplebatterynotifier/util/AppPrefs.java @@ -60,7 +60,7 @@ private AppPrefs() { * * @return the configured critical level */ - public static int criticalLevel(final Context context) { + public static int criticalLevel(Context context) { return prefs(context).getInt(context.getString(R.string._pref_key_critical_battery_level), DEFAULT_CRITICAL_LEVEL); } @@ -72,7 +72,7 @@ public static int criticalLevel(final Context context) { * * @return the configured warning level */ - public static int warningLevel(final Context context) { + public static int warningLevel(Context context) { return prefs(context).getInt(context.getString(R.string._pref_key_warn_battery_level), DEFAULT_WARNING_LEVEL); } @@ -84,7 +84,7 @@ public static int warningLevel(final Context context) { * * @return the configured {@code (critical, warning)} thresholds */ - public static LevelThresholds batteryLevels(final Context context) { + public static LevelThresholds batteryLevels(Context context) { return new LevelThresholds(criticalLevel(context), warningLevel(context)); } @@ -95,7 +95,7 @@ public static LevelThresholds batteryLevels(final Context context) { * @param context Application context * @param levels the thresholds to store */ - public static void setBatteryLevels(final Context context, final LevelThresholds levels) { + public static void setBatteryLevels(Context context, LevelThresholds levels) { prefs(context).edit() .putInt(context.getString(R.string._pref_key_critical_battery_level), levels.critical()) .putInt(context.getString(R.string._pref_key_warn_battery_level), levels.warning()) @@ -112,7 +112,7 @@ public static void setBatteryLevels(final Context context, final LevelThresholds * * @return the configured limit in %/h */ - public static int drainLimitPph(final Context context) { + public static int drainLimitPph(Context context) { return clampDrainLimit(prefs(context).getInt( context.getString(R.string._pref_key_fast_drain_limit), DEFAULT_DRAIN_LIMIT_PPH)); } @@ -126,7 +126,7 @@ public static int drainLimitPph(final Context context) { * * @return the limit clamped to {@code [MIN_DRAIN_LIMIT_PPH, MAX_DRAIN_LIMIT_PPH]} */ - public static int clampDrainLimit(final int stored) { + public static int clampDrainLimit(int stored) { return Math.max(MIN_DRAIN_LIMIT_PPH, Math.min(MAX_DRAIN_LIMIT_PPH, stored)); } @@ -143,7 +143,7 @@ public static boolean vibrateEnabled(Context context) { return prefs(context).getBoolean(context.getString(R.string._pref_key_notifications_vibrate), DEFAULT_VIBRATE); } - private static SharedPreferences prefs(final Context context) { + private static SharedPreferences prefs(Context context) { return PreferenceManager.getDefaultSharedPreferences(context); } }