java-code-review

Name: java-code-review
Author: decebals

686stars133forks

How to install java-code-review?

decebals/claude-code-java/java-code-review

$npx -y skills add decebals/claude-code-java --skill java-code-review

Installs into the current project.

Summary

Systematic code review for Java with null safety, exception handling, concurrency, and performance checks. Use when user says "review code", "check this PR", "code review", or before merging changes.

Files · 1

View on GitHub

SKILL.md

1	# Java Code Review Skill
2
3	Systematic code review checklist for Java projects.
4
5	## When to Use
6	- User says "review this code" / "check this PR" / "code review"
7	- Before merging a PR
8	- After implementing a feature
9
10	## Review Strategy
11
12	1. Quick scan - Understand intent, identify scope
13	2. Checklist pass - Go through each category below
14	3. Summary - List findings by severity (Critical → Minor)
15
16	## Output Format
17
18	```markdown
19	## Code Review: [file/feature name]
20
21	### Critical
22	- [Issue description + line reference + suggestion]
23
24	### Improvements
25	- [Suggestion + rationale]
26
27	### Minor/Style
28	- [Nitpicks, optional improvements]
29
30	### Good Practices Observed
31	- [Positive feedback - important for morale]
32	```
33
34	---
35
36	## Review Checklist
37
38	### 1. Null Safety
39
40	Check for:
41	```java
42	// ❌ NPE risk
43	String name = user.getName().toUpperCase();
44
45	// ✅ Safe
46	String name = Optional.ofNullable(user.getName())
47	.map(String::toUpperCase)
48	.orElse("");
49
50	// ✅ Also safe (early return)
51	if (user.getName() == null) {
52	return "";
53	}
54	return user.getName().toUpperCase();
55	```
56
57	Flags:
58	- Chained method calls without null checks
59	- Missing `@Nullable` / `@NonNull` annotations on public APIs
60	- `Optional.get()` without `isPresent()` check
61	- Returning `null` from methods that could return `Optional` or empty collection
62
63	Suggest:
64	- Use `Optional` for return types that may be absent
65	- Use `Objects.requireNonNull()` for constructor/method params
66	- Return empty collections instead of null: `Collections.emptyList()`
67
68	### 2. Exception Handling
69
70	Check for:
71	```java
72	// ❌ Swallowing exceptions
73	try {
74	process();
75	} catch (Exception e) {
76	// silently ignored
77	}
78
79	// ❌ Catching too broad
80	catch (Exception e) { }
81	catch (Throwable t) { }
82
83	// ❌ Losing stack trace
84	catch (IOException e) {
85	throw new RuntimeException(e.getMessage());
86	}
87
88	// ✅ Proper handling
89	catch (IOException e) {
90	log.error("Failed to process file: {}", filename, e);
91	throw new ProcessingException("File processing failed", e);
92	}
93	```
94
95	Flags:
96	- Empty catch blocks
97	- Catching `Exception` or `Throwable` broadly
98	- Losing original exception (not chaining)
99	- Using exceptions for flow control
100	- Checked exceptions leaking through API boundaries
101
102	Suggest:
103	- Log with context AND stack trace
104	- Use specific exception types
105	- Chain exceptions with `cause`
106	- Consider custom exceptions for domain errors
107
108	### 3. Collections & Streams
109
110	Check for:
111	```java
112	// ❌ Modifying while iterating
113	for (Item item : items) {
114	if (item.isExpired()) {
115	items.remove(item); // ConcurrentModificationException
116	}
117	}
118
119	// ✅ Use removeIf
120	items.removeIf(Item::isExpired);
121
122	// ❌ Stream for simple operations
123	list.stream().forEach(System.out::println);
124
125	// ✅ Simple loop is cleaner
126	for (Item item : list) {
127	System.out.println(item);
128	}
129
130	// ❌ Collecting to modify
131	List<String> names = users.stream()
132	.map(User::getName)
133	.collect(Collectors.toList());
134	names.add("extra"); // Might be immutable!
135
136	// ✅ Explicit mutable list
137	List<String> names = users.stream()
138	.map(User::getName)
139	.collect(Collectors.toCollection(ArrayList::new));
140	```
141
142	Flags:
143	- Modifying collections during iteration
144	- Overusing streams for simple operations
145	- Assuming `Collectors.toList()` returns mutable list
146	- Not using `List.of()`, `Set.of()`, `Map.of()` for immutable collections
147	- Parallel streams without understanding implications
148
149	Suggest:
150	- `List.copyOf()` for defensive copies
151	- `removeIf()` instead of iterator removal
152	- Streams for transformations, loops for side effects
153
154	### 4. Concurrency
155
156	Check for:
157	```java
158	// ❌ Not thread-safe
159	private Map<String, User> cache = new HashMap<>();
160
161	// ✅ Thread-safe
162	private Map<String, User> cache = new ConcurrentHashMap<>();
163
164	// ❌ Check-then-act race condition
165	if (!map.containsKey(key)) {
166	map.put(key, computeValue());
167	}
168
169	// ✅ Atomic operation
170	map.computeIfAbsent(key, k -> computeValue());
171
172	// ❌ Double-checked locking (broken without volatile)
173	if (instance == null) {
174	synchronized(this) {
175	if (instance == null) {
176	instance = new Instance();
177	}
178	}
179	}
180	```
181
182	Flags:
183	- Shared mutable state without synchronization
184	- Check-then-act patterns without atomicity
185	- Missing `volatile` on shared variables
186	- Synchronized on non-final objects
187	- Thread-unsafe lazy initialization
188
189	Suggest:
190	- Prefer immutable objects
191	- Use `java.util.concurrent` classes
192	- `AtomicReference`, `AtomicInteger` for simple cases
193	- Consider `@ThreadSafe` / `@NotThreadSafe` annotations
194
195	### 5. Java Idioms
196
197	equals/hashCode:
198	```java
199	// ❌ Only equals without hashCode
200	@Override
201	public boolean equals(Object o) { ... }
202	// Missing hashCode!
203
204	// ❌ Mutable fields in hashCode
205	@Override
206	public int hashCode() {
207	return Objects.ha

People who liked this also viewed

6 picks

Type

java-code-review

686stars133forks

View on GitHub

How to install java-code-review?

decebals/claude-code-java/java-code-review

$npx -y skills add decebals/claude-code-java --skill java-code-review

Installs into the current project.

Summary

Systematic code review for Java with null safety, exception handling, concurrency, and performance checks. Use when user says "review code", "check this PR", "code review", or before merging changes.

Files · 1

View on GitHub

SKILL.md

1	# Java Code Review Skill
2
3	Systematic code review checklist for Java projects.
4
5	## When to Use
6	- User says "review this code" / "check this PR" / "code review"
7	- Before merging a PR
8	- After implementing a feature
9
10	## Review Strategy
11
12	1. Quick scan - Understand intent, identify scope
13	2. Checklist pass - Go through each category below
14	3. Summary - List findings by severity (Critical → Minor)
15
16	## Output Format
17
18	```markdown
19	## Code Review: [file/feature name]
20
21	### Critical
22	- [Issue description + line reference + suggestion]
23
24	### Improvements
25	- [Suggestion + rationale]
26
27	### Minor/Style
28	- [Nitpicks, optional improvements]
29
30	### Good Practices Observed
31	- [Positive feedback - important for morale]
32	```
33
34	---
35
36	## Review Checklist
37
38	### 1. Null Safety
39
40	Check for:
41	```java
42	// ❌ NPE risk
43	String name = user.getName().toUpperCase();
44
45	// ✅ Safe
46	String name = Optional.ofNullable(user.getName())
47	.map(String::toUpperCase)
48	.orElse("");
49
50	// ✅ Also safe (early return)
51	if (user.getName() == null) {
52	return "";
53	}
54	return user.getName().toUpperCase();
55	```
56
57	Flags:
58	- Chained method calls without null checks
59	- Missing `@Nullable` / `@NonNull` annotations on public APIs
60	- `Optional.get()` without `isPresent()` check
61	- Returning `null` from methods that could return `Optional` or empty collection
62
63	Suggest:
64	- Use `Optional` for return types that may be absent
65	- Use `Objects.requireNonNull()` for constructor/method params
66	- Return empty collections instead of null: `Collections.emptyList()`
67
68	### 2. Exception Handling
69
70	Check for:
71	```java
72	// ❌ Swallowing exceptions
73	try {
74	process();
75	} catch (Exception e) {
76	// silently ignored
77	}
78
79	// ❌ Catching too broad
80	catch (Exception e) { }
81	catch (Throwable t) { }
82
83	// ❌ Losing stack trace
84	catch (IOException e) {
85	throw new RuntimeException(e.getMessage());
86	}
87
88	// ✅ Proper handling
89	catch (IOException e) {
90	log.error("Failed to process file: {}", filename, e);
91	throw new ProcessingException("File processing failed", e);
92	}
93	```
94
95	Flags:
96	- Empty catch blocks
97	- Catching `Exception` or `Throwable` broadly
98	- Losing original exception (not chaining)
99	- Using exceptions for flow control
100	- Checked exceptions leaking through API boundaries
101
102	Suggest:
103	- Log with context AND stack trace
104	- Use specific exception types
105	- Chain exceptions with `cause`
106	- Consider custom exceptions for domain errors
107
108	### 3. Collections & Streams
109
110	Check for:
111	```java
112	// ❌ Modifying while iterating
113	for (Item item : items) {
114	if (item.isExpired()) {
115	items.remove(item); // ConcurrentModificationException
116	}
117	}
118
119	// ✅ Use removeIf
120	items.removeIf(Item::isExpired);
121
122	// ❌ Stream for simple operations
123	list.stream().forEach(System.out::println);
124
125	// ✅ Simple loop is cleaner
126	for (Item item : list) {
127	System.out.println(item);
128	}
129
130	// ❌ Collecting to modify
131	List<String> names = users.stream()
132	.map(User::getName)
133	.collect(Collectors.toList());
134	names.add("extra"); // Might be immutable!
135
136	// ✅ Explicit mutable list
137	List<String> names = users.stream()
138	.map(User::getName)
139	.collect(Collectors.toCollection(ArrayList::new));
140	```
141
142	Flags:
143	- Modifying collections during iteration
144	- Overusing streams for simple operations
145	- Assuming `Collectors.toList()` returns mutable list
146	- Not using `List.of()`, `Set.of()`, `Map.of()` for immutable collections
147	- Parallel streams without understanding implications
148
149	Suggest:
150	- `List.copyOf()` for defensive copies
151	- `removeIf()` instead of iterator removal
152	- Streams for transformations, loops for side effects
153
154	### 4. Concurrency
155
156	Check for:
157	```java
158	// ❌ Not thread-safe
159	private Map<String, User> cache = new HashMap<>();
160
161	// ✅ Thread-safe
162	private Map<String, User> cache = new ConcurrentHashMap<>();
163
164	// ❌ Check-then-act race condition
165	if (!map.containsKey(key)) {
166	map.put(key, computeValue());
167	}
168
169	// ✅ Atomic operation
170	map.computeIfAbsent(key, k -> computeValue());
171
172	// ❌ Double-checked locking (broken without volatile)
173	if (instance == null) {
174	synchronized(this) {
175	if (instance == null) {
176	instance = new Instance();
177	}
178	}
179	}
180	```
181
182	Flags:
183	- Shared mutable state without synchronization
184	- Check-then-act patterns without atomicity
185	- Missing `volatile` on shared variables
186	- Synchronized on non-final objects
187	- Thread-unsafe lazy initialization
188
189	Suggest:
190	- Prefer immutable objects
191	- Use `java.util.concurrent` classes
192	- `AtomicReference`, `AtomicInteger` for simple cases
193	- Consider `@ThreadSafe` / `@NotThreadSafe` annotations
194
195	### 5. Java Idioms
196
197	equals/hashCode:
198	```java
199	// ❌ Only equals without hashCode
200	@Override
201	public boolean equals(Object o) { ... }
202	// Missing hashCode!
203
204	// ❌ Mutable fields in hashCode
205	@Override
206	public int hashCode() {
207	return Objects.ha

People who liked this also viewed

6 picks

Type