[JUGNsk #23] Тагир Валеев: "А вы никогда не думали писать код без багов?"

В докладе Тагир рассмотрит некоторые типичные ошибки, которые совершают Java разработчики, расскажет какие инструменты и подходы стоит использовать, чтобы избежать подобных проблем в своем коде. И будет немного про AI, куда же без него!

Transcript

1. А вы никогда не думали писать код без багов? Тагир Валеев 1
3. About me ✔15 years of Java programming experience ✔7 years in JetBrains (IntelliJ IDEA Java team technical lead) ✔Contributed to FindBugs static analyzer (~2014) ✔OpenJDK committer ✔Java Champion ✔Wrote a book about Java mistakes 3
4. 4 https://www.manning.com/books/100- java-mistakes-and-how-to-avoid-them https://www.amazon.com/dp/1633437965/
5. 5 Bugs Huge and complex Tiny and local
7. How to avoid bugs? 7 ✔Code style ✔Idiomatic code ✔Avoid repetitions ✔Code review ✔Pair programming ✔Assertions ✔AI! ✔Static analysis ✔Dynamic analysis (Pathfinder) ✔Testing ✔Unit tests ✔+ Coverage control (JaCoCo) ✔+ Mutation coverage control (Pitest) ✔Smoke tests ✔Functional tests ✔Integration tests ✔Property tests ✔…
8. Bad news: there’s no silver bullet 8
10. Good news: Swiss cheese model 10 James Reason, 1990 https://en.wikipedia.org/wiki/Swiss_cheese_model
11. Initial capacity (StringBuilder) 11 static String indentString(String str, int indent) { int capacity = str.length() + indent < 0 ? 0 : indent; StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); return sb.toString(); }
12. Initial capacity 12 static String indentString(String str, int indent) { int capacity = str.length() + indent < 0 ? 0 : indent; StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); return sb.toString(); }
13. Initial capacity 13 static String indentString(String str, int indent) { int capacity = str.length() + indent < 0 ? 0 : indent; StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); return sb.toString(); } 1. indent >= 0 → capacity = indent >= 0 1. -str.length() <= indent < 0 → capacity = indent < 0 1. indent < -str.length() → capacity = 0
14. Let’s ask AI 14
18. What’s wrong here? 18 static String indentString(String str, int indent) { int capacity = str.length() + (indent < 0 ? 0 : indent); StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); return sb.toString(); }
19. What’s wrong here? 19 static String indentString(String str, int indent) { int capacity = str.length() + indent < 0 ? 0 : indent; StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); return sb.toString(); } ✔Initial capacity is not easily testable! ✔Manual algorithm implementation ✔Action at a distance ✔Premature optimization?
20. Asserts to the rescue? 20 ✔Initial capacity is not easily testable! ✔Manual algorithm implementation ✔Action at a distance ✔Premature optimization? static String indentString(String str, int indent) { int capacity = str.length() + indent < 0 ? 0 : indent; StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); assert capacity == sb.length(); return sb.toString(); }
21. How to fix? 21 static String indentString(String str, int indent) { int capacity = str.length() + Math.max(0, indent); StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); return sb.toString(); } ✔Initial capacity is not easily testable! ✔Manual algorithm implementation ✔Action at a distance ✔Premature optimization?
22. What’s wrong here? 22 static String indentStringAndLineBreak(String str, int indent) { int capacity = str.length() + Math.max(0, indent) + 1; StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); sb.append('\n'); return sb.toString(); } ✔Initial capacity is not easily testable! ✔Manual algorithm implementation ✔Action at a distance ✔Premature optimization?
23. How to fix? 23 static String indentString(String str, int indent) { if (indent <= 0) return str; int capacity = str.length() + indent; StringBuilder sb = new StringBuilder(capacity); for (int i = 0; i < indent; i++) { sb.append(' '); } sb.append(str); return sb.toString(); } ✔Initial capacity is not easily testable! ✔Manual algorithm implementation ✔Action at a distance ✔Premature optimization?
24. Or probably this way? 24 static String indentString(String str, int indent) { if (indent <= 0) return str; return " ".repeat(indent) + str; } ✔Initial capacity is not easily testable! ✔Manual algorithm implementation ✔Action at a distance ✔Premature optimization?
25. What’s faster? 25 str indent No capacity (ns) Capacity (ns) String.repeat (ns) "s" 1 18.1 20.0 "very…long" 1 32.7 30.2 "你好" 1 26.4 27.0 "s" 10 24.7 26.6 "very…long" 10 37.3 35.7 "你好" 10 34.5 38.6 "s" 100 113.4 104.2 "very…long" 100 143.2 108.4 "你好" 100 203.6 193.8
26. What’s faster? 26 str indent No capacity (ns) Capacity (ns) String.repeat (ns) "s" 1 18.1 20.0 6.0 "very…long" 1 32.7 30.2 14.4 "你好" 1 26.4 27.0 9.2 "s" 10 24.7 26.6 15.4 "very…long" 10 37.3 35.7 20.6 "你好" 10 34.5 38.6 16.2 "s" 100 113.4 104.2 27.5 "very…long" 100 143.2 108.4 36.9 "你好" 100 203.6 193.8 34.2
27. Initial capacity (ArrayList) 27 List<String> trimAndAdd(List<String> input, String newItem) { List<String> result = new ArrayList<>(input.size() + newItem == null ? 0 : 1); for (String s : input) { result.add(s.trim()); } if (newItem != null) { result.add(newItem.trim()); } return result; }
28. Precedence again 28 List<String> trimAndAdd(List<String> input, String newItem) { List<String> result = new ArrayList<>(input.size() + newItem == null ? 0 : 1); for (String s : input) { result.add(s.trim()); } if (newItem != null) { result.add(newItem.trim()); } return result; }
29. Static analysis cheese slice helps 29
30. Write tests! import java.util.Set; class Utils { static String makeUniqueId(String id, Set<String> usedIds) { int i = 1; String uniqueId = id; while (usedIds.contains(uniqueId)) { uniqueId = id + "_" + i; } return uniqueId; } } 30
31. AI can do this! 31
32. public final class UtilsTest { @Test public void testMakeUniqueId() { String id = "Test"; Set<String> usedIds = new HashSet<>(); String newId = Utils.makeUniqueId(id, usedIds); assertEquals(id, newId); } @Test public void testMakeUniqueIdWithPresentId() { String id = "Test"; Set<String> usedIds = new HashSet<>(); usedIds.add(id); String newId = Utils.makeUniqueId(id, usedIds); assertNotEquals(id, newId); } } 32
33. Check coverage 33
34. Oops! @Test public void testMakeUniqueIdWithTwoConflicts() { String newId = Utils.makeUniqueId("Test", Set.of("Test", "Test_1")); assertEquals("Test_2", newId); } 34
35. Oops! @Test public void testMakeUniqueIdWithTwoConflicts() { String newId = Utils.makeUniqueId("Test", Set.of("Test", "Test_1")); assertEquals("Test_2", newId); } 35
36. @Test public void testMakeUniqueIdWithTwoConflicts() { String newId = Utils.makeUniqueId("Test", Set.of("Test", "Test_1")); assertEquals("Test_2", newId); } import java.util.Set; class Utils { static String makeUniqueId(String id, Set<String> usedIds) { int i = 1; String uniqueId = id; while (usedIds.contains(uniqueId)) { uniqueId = id + "_" + i; } return uniqueId; } } 36
37. import java.util.Set; class Utils { static String makeUniqueId(String id, Set<String> usedIds) { int i = 1; String uniqueId = id; while (usedIds.contains(uniqueId)) { uniqueId = id + "_" + i; } return uniqueId; } } Idempotent loop body 1. The condition is false initially. The loop is not executed at all. 2. The condition is true initially, but the body makes it false. The loop is executed only once. 3. The condition is true initially, and the body doesn’t change it. The loop is infinite, and the program hangs. 37
38. Static analysis helps here! 38
39. Rule: If you have a loop, your tests should test at least cases with 0, 1, and 2+ iterations. 39
40. Corner cases static void printInclusive(int from, int to) { for (int i = from; i <= to; i++) { System.out.println(i); } } 40
41. AI to the rescue? 41
42. AI to the rescue? static void printInclusive(int from, int to) { for (int i = from; i <= to; i++) { System.out.println(i); } } 42
43. Let’s protect ourselves static void printInclusive(int from, int to) { if (from > to) { throw new IllegalArgumentException("from > to"); } if (to - from < 0 || to - from > 1000) { throw new IllegalArgumentException("too many numbers to process"); } for (int i = from; i <= to; i++) { System.out.println(i); } } 43
44. Are there still mistakes? static void printInclusive(int from, int to) { if (from > to) { throw new IllegalArgumentException("from > to"); } if (to - from < 0 || to - from > 1000) { throw new IllegalArgumentException("too many numbers to process"); } for (int i = from; i <= to; i++) { System.out.println(i); } } 44
45. Hm… static void printInclusive(int from, int to) { if (from > to) { throw new IllegalArgumentException("from > to"); } if (to - from < 0 || to - from > 1000) { throw new IllegalArgumentException("too many numbers to process"); } for (int i = from; i <= to; i++) { System.out.println(i); } } Bullshit: printInclusive(-2_000_000_000, 2_000_000_000); More null-checks for the null-check god Doubtful but okaaaay OMG C’mon it’s just a sample Are you crazy? 45
46. Oops static void printInclusive(int from, int to) { if (from > to) { throw new IllegalArgumentException("from > to"); } if (to - from < 0 || to - from > 1000) { throw new IllegalArgumentException("too many numbers to process"); } for (int i = from; i <= to; i++) { System.out.println(i); } } public static void main(String[] args) { printInclusive(Integer.MAX_VALUE - 10, Integer.MAX_VALUE); } 46
47. Oops static void printInclusive(int from, int to) { if (from > to) { throw new IllegalArgumentException("from > to"); } if (to - from > 1000) { throw new IllegalArgumentException("too many numbers to process"); } for (int i = from; i <= to; i++) { System.out.println(i); } } public static void main(String[] args) { printInclusive(Integer.MAX_VALUE - 10, Integer.MAX_VALUE); } 47
48. How to fix it? static void printInclusive(int from, int to) { if (from > to) { throw new IllegalArgumentException("from > to"); } if (to - from < 0 || to - from > 1000) { throw new IllegalArgumentException("too many numbers to process"); } for (int i = from; i >= from && i <= to; i++) { System.out.println(i); } } public static void main(String[] args) { printInclusive(Integer.MAX_VALUE - 10, Integer.MAX_VALUE); } 48
49. Real life https://github.com/JetBrains/intellij-community/commit/b56a1fc16d446c92528994e0d333cbfa9ade21cd 49
50. Use Stream API! 50
51. Convert to Kotlin! (Ctrl+Alt+Shift+K) fun printInclusive(from: Int, to: Int) { require(from <= to) { "from > to" } require(to - from < 0 || to - from <= 1000) { "too many numbers to process" } for (i in from..to) { println(i) } } @JvmStatic fun main(args: Array<String>) { printInclusive(Int.MAX_VALUE - 10, Int.MAX_VALUE) } Output: 2147483637 2147483638 2147483639 2147483640 2147483641 2147483642 2147483643 2147483644 2147483645 2147483646 2147483647 51
52. Rule: You should have a language construct or library method for every useful and repeating task Task: iterate over closed range of numbers IntStream.rangeClosed(from, to) for (i in from..to) { … } for (int i = from; i <= to; i++) { … } 52
53. Clamping values static void printProgress(int percent) { if (percent > 100) { percent = 100; } if (percent < 0) { percent = 0; } System.out.println("Progress: " + percent + "%"); } 53
54. Clamping values static void printProgress(int percent) { percent = percent < 0 ? 0 : percent > 100 ? 100 : percent; System.out.println("Progress: " + percent + "%"); } 54
55. Clamping values static void printProgress(int percent) { percent = Math.max(Math.min(percent, 0), 100); System.out.println("Progress: " + percent + "%"); } 55
56. Clamping values static void printProgress(int percent) { percent = Math.max(Math.min(percent, 0), 100); System.out.println("Progress: " + percent + "%"); } public static void main(String[] args) { printProgress(-10); printProgress(10); printProgress(50); printProgress(90); printProgress(130); } Output: Progress: 100% Progress: 100% Progress: 100% Progress: 100% Progress: 100% 56
57. Clamping values 57
58. AI to the rescue! 58
59. In the wild https://github.com/jenkinsci/jenkins/commit/e00f99251e0b 59
60. Real solution (Java 21) 60
61. Real solution (Java 21) static void printProgress(int percent) { percent = Math.clamp(percent, 0, 100); System.out.println("Progress: " + percent + "%"); } public static void main(String[] args) { printProgress(-10); printProgress(10); printProgress(50); printProgress(90); printProgress(130); } 61
62. How many rows do you need? 62 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁
63. How many rows do you need? static int getRowCount(int count, int columns) { return (count + columns - 1) / columns; } 63 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁
64. How many rows do you need? static int getRowCount(int count, int columns) { return (count + columns - 1) / columns; } System.out.println(getRowCount(17, 6)); 3 System.out.println(getRowCount(18, 6)); 3 System.out.println(getRowCount(19, 6)); 4 System.out.println(getRowCount(Integer.MAX_VALUE, 6)); -357913940 64 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁
65. How many rows do you need? static int getRowCount(int count, int columns) { return Math.floorDiv(count - 1, columns) + 1; } 65 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁
66. How many rows do you need (Java 18+)? static int getRowCount(int count, int columns) { return Math.ceilDiv(count, columns); } 66 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁🎁🎁🎁🎁 🎁🎁
67. Poor man template 67 private static final String TEMPLATE = "Hello USER_NAME!"; static void greetUser(String user) { String greeting = TEMPLATE.replaceAll("USER_NAME", user); System.out.println(greeting); }
68. Poor man template 68 private static final String TEMPLATE = "Hello USER_NAME!"; static void greetUser(String user) { String greeting = TEMPLATE.replaceAll("USER_NAME", user); System.out.println(greeting); } greetUser("John"); Hello John!
69. Poor man template 69 private static final String TEMPLATE = "Hello USER_NAME!"; static void greetUser(String user) { String greeting = TEMPLATE.replaceAll("USER_NAME", user); System.out.println(greeting); } greetUser("$1lly name"); Exception in thread "main" java.lang.IndexOutOfBoundsException: No group 1 at java.base/java.util.regex.Matcher.checkGroup(Matcher.java:1818) at java.base/java.util.regex.Matcher.start(Matcher.java:496) at java.base/java.util.regex.Matcher.appendExpandedReplacement(Matcher.java:1107) at java.base/java.util.regex.Matcher.appendReplacement(Matcher.java:1014) at java.base/java.util.regex.Matcher.replaceAll(Matcher.java:1200) at java.base/java.lang.String.replaceAll(String.java:3065) at com.example.Utils.greetUser(Utils.java:8) at com.example.Utils.main(Utils.java:13)
70. Convenience comes at a cost 70 String.replace: replaces all substrings String.replaceAll: replaces all regular expression matches public String replaceAll(String regex, String replacement) { return Pattern.compile(regex).matcher(this).replaceAll(replacement); } Rules: when designing your API, note that too many “convenient” methods may make things confusing. Avoid stringly-typed code.
71. Stringly-typed code 71 static int countPathComponents(String fileName) { String[] components = fileName.split(File.separator); return (int) Stream.of(components) .filter(Predicate.not(String::isEmpty)).count(); } countPathComponents("/etc/passwd") 🡪 2
72. On Windows machine 72 static int countPathComponents(String fileName) { String[] components = fileName.split(File.separator); return (int) Stream.of(components) .filter(Predicate.not(String::isEmpty)).count(); } countPathComponents("C:\\tmp\\file.txt") Exception in thread "main" java.util.regex.PatternSyntaxException: Unescaped trailing backslash near index 1 \ at java.base/java.util.regex.Pattern.error(Pattern.java:2204) at java.base/java.util.regex.Pattern.compile(Pattern.java:1951) at java.base/java.util.regex.Pattern.<init>(Pattern.java:1576) at java.base/java.util.regex.Pattern.compile(Pattern.java:1101) at java.base/java.lang.String.split(String.java:3352) at java.base/java.lang.String.split(String.java:3443) at com.example.Utils.countPathComponents(Utils.java:12) at com.example.Utils.main(Utils.java:21)
73. Fix? 73 static int countPathComponents(String fileName) { return (int) Pattern.compile(File.separator, Pattern.LITERAL) .splitAsStream(fileName) .filter(Predicate.not(String::isEmpty)).count(); } countPathComponents("C:\\tmp\\file.txt")
74. Idiomatic? 74 static int countPathComponents(String fileName) { return Path.of(fileName).getNameCount(); }
75. Idiomatic! 75 static int countPathComponents(String fileName) { return Path.of(fileName).getNameCount(); } Rule (again): use proper types for your entities, and strong typing will help you to avoid bugs
76. Low-level stuff is dangerous 76 static long clearHighBits(long value) { return value & 0x0000_0000_FFFF_FFFF; } public static void main(String[] args) { System.out.printf("%x", clearHighBits(0x1234_5678_9ABC_DEF0L)); }
78. Low-level stuff is dangerous 78 static long clearHighBits(long value) { return value & 0x0000_0000_FFFF_FFFF; } public static void main(String[] args) { System.out.printf("%x", clearHighBits(0x1234_5678_9ABC_DEF0L)); } 123456789abcdef0
79. Low-level stuff is dangerous 79 static long clearHighBits(long value) { return value & 0x0000_0000_FFFF_FFFF; // == -1 == 0xFFFF_FFFF_FFFF_FFFFL } public static void main(String[] args) { System.out.printf("%x", clearHighBits(0x1234_5678_9ABC_DEF0L)); } 123456789abcdef0
80. Fixed 80 static long clearHighBits(long value) { return value & 0x0000_0000_FFFF_FFFFL; } public static void main(String[] args) { System.out.printf("%x", clearHighBits(0x1234_5678_9ABC_DEF0L)); } 123456789abcdef0
81. Static analysis helps 81
82. Keep lower bits 82 static int keepLowerBits(int value, int bitsToKeep) { return value & ((1 << bitsToKeep) - 1); } public static void main(String[] args) { System.out.printf("%x%n", keepLowerBits(0x1234_5678, 0)); → 0 System.out.printf("%x%n", keepLowerBits(0x1234_5678, 8)); → 78 System.out.printf("%x%n", keepLowerBits(0x1234_5678, 16)); → 5678 System.out.printf("%x%n", keepLowerBits(0x1234_5678, 24)); → 345678 }
83. Corner case 83 static int keepLowerBits(int value, int bitsToKeep) { return value & ((1 << bitsToKeep) - 1); } public static void main(String[] args) { System.out.printf("%x%n", keepLowerBits(0x1234_5678, 0)); → 0 System.out.printf("%x%n", keepLowerBits(0x1234_5678, 8)); → 78 System.out.printf("%x%n", keepLowerBits(0x1234_5678, 16)); → 5678 System.out.printf("%x%n", keepLowerBits(0x1234_5678, 24)); → 345678 System.out.printf("%x%n", keepLowerBits(0x1234_5678, 32)); → 0 }
84. 84 Wrong Useful Noise
85. 85 Rules: Do not invent algorithms, use libraries Do not invent algorithms, read books (Hacker’s Delight) If you need to invent an algorithm, test it thoroughly Pay attention to corner cases
86. Conclusion ✔Use static analysis ✔Write unit tests ✔Clear constructs for every idiom ✔Avoid repetitions ✔Consult AI when in doubt but don’t rely on it too much ✔No premature optimizations ✔Educate yourself, read books ☺ 86
87. Thank you 87