Code Smells @Voxxed Bucharest 24

Transcript

1. VictorRentea.ro 49 Code Smells by Victor Rentea
2. 👋 I'm Victor Rentea 🇷🇴 Java Champion, PhD(CS) 18 years of coding 10 years of training & consul2ng at 130+ companies: ❤ Refactoring, Architecture, Unit Tes4ng 🛠 Spring, Hibernate, Reac4ve ⚡ Java Performance & Secure Coding Lead of European So8ware Cra8ers (6.900)👉 victorrentea.ro/community Mee4ng online every month from 1700 CET Channel: YouTube.com/vrentea Life += 👰 + 👧 + 🙇 + 🐈 @victorrentea 🇷🇴 past events 👉 victorrentea.ro/catalog
3. 51 VictorRentea.ro a training by §Method length < ...... lines of code §Method depth < .... indenta3on levels §File length < ....... lines §Feeling about //comments in code: ....... §When we see duplicated code: ...... §To work with collec3ons, instead of a for loop, use: ....... §Feeling about seAers: .... §FormaBng: for ( var e : list ) 🥾Kick-off Quiz 20* 3 200 not required by good code extract a shared method .map / .filter avoid - prefer immutable objects! auto-format by IDE
4. VictorRentea.ro 52 Code Smells code paAerns that hurt you later “If it stinks, you have to change it.”
5. VictorRentea.ro 53 Code Smells Copy-Paste Programming DRY Don't Repeat Yourself
6. VictorRentea.ro 54 Too DRY 🦂 Removing duplica0on too aggressively can: - Increase complexity: godMethod(a, b, c, d, false, true, -1, (x,y) -> ...) - Increase coupling: method uses 10 dependencies - Abuse inheritance: class Green extends Color extends Base extends ... - Accidental Coupling☣ of unrelated flows with similar code Only unify code that changes together 👉 ask PO The rule of 3: a li<le repe00on is beKer than the wrong abstrac>ons
7. 55 VictorRentea.ro a training by There are only two things hard in programming ... 1) cache invalida*on 2) naming things 3) and off by one errors A good name proves it does one clear thing SRP
8. VictorRentea.ro 57 Code Smells Bad Names
9. VictorRentea.ro 58 Command-Query Separation Violation price = computePrice(..); // INSERT in DB?! Bad Names Code Smells checkCustomer(customer); // updating customer -> { ... 20 lines of code } // anonymous logic updatePlace(player, ..); // purely technical, lacks intent MovieType priceCode; // synonyms confuse😵💫 dr // mysterious acronym
10. VictorRentea.ro 60 Also see: FizzBuzz Enterpise Edi4on 🤣🤣🤣 Code Smells Use simple memorable names from the problem domain Names with bad Signal/Noise ratio Window Door TV Couch Remote Painting
11. 61 VictorRentea.ro a training by Con2nuous Dis2la2on Rename
12. VictorRentea.ro 63 Stuff That Starts Small... Code Smells ß 5 lines of code 😇 then 7, 10, 15, 25, 45...
13. VictorRentea.ro 64 Monster Method God Class Many Parameters > 20 lines⛔ > 200 lines⛔ > 4 ⛔ Bloaters The more complex, the shorter depth > 3⛔ Complex Lambda -> { ... è Extract reusable Parameter Object Code Smells è Split in Flow A / Flow B è Split method by SRP è Split in High / Low level Extract if you can find a good name The magic number 7±2 How many things the human brain can keep in working memory (wiki) max 7 ideas/method max 7 methods/class Same Level of AbstracKon (SLAb) also: Generics<A,B,U,S,E>
14. 71 VictorRentea.ro a training by When in Doubt, Extract a Method
15. VictorRentea.ro 75 Code Smells related data elements moving around together Missing Type String int int String
16. VictorRentea.ro 76 Map<Long, List<Long>> è List<CustomerOrders> Data Clumps Create more classes to group data that s4cks together int, int è Interval Tuple4<String, Long, Long, Date> è PricedProduct Lightweight structures Java: @Data/@Value, record Kotlin: data class Scala: case class C#: record class TS: class, interface, type Code Smells aka: {"amount":10, "cur": "EUR"} è Money{amount,currency} js, ts, php, py.. Interval(start,end) ArrayGeddon What fields I receive?😱 IDE Refactoring = unsafe! Missing Type Missing Abstrac7on Tangled Tuples
17. 77 VictorRentea.ro a training by Discover New Classes Simplify Code Shrink En==es carModel.years:Interval Less Parameters in(x, Interval) Interval(start,end) Guard Constraints start<end Host Bits of Behavior interval.intersects(..) If Objects, not only a Data Structure
18. 78 VictorRentea.ro a training by Code Smells
19. VictorRentea.ro 80 Code Smells class state (fields) logicLogic using the state of another object Feature Envy
20. VictorRentea.ro 81 Logic using the state of another object !!... int place = player.getPlace() + roll; if (place !>= 12) { place -= 12; } player.setPlace(place); !!... Data Classes Anemic class with fields, but no behavior Feature Envy Code Smells Rich Objects with logic & constraints help simplify core logic Keep behavior next to state (OOP) player.advance(roll);
21. VictorRentea.ro 86 public Customer findById(id) { return repo.findById(id); } int startYear() { return yearInterval.start(); } Middle Man Indirec4on without abstrac4on Code Smells NOISE
22. VictorRentea.ro 87 confused by raw data without meaning Map<Long, List<Long>> map Map<Long, List<Long>> customerIdToOrderIds void redeemCoupon(Long couponId, Long customerId, String email) Code Smells redeemCoupon(dto.cust, dto.coupon, dto.email) Caller: Primi%ve Obsession Declara;on: Tuple3<Long, String, List<Long>> What do these mean? 😨 one of: EMEA|NA|LATA|APAC
23. 88 VictorRentea.ro a training by void redeemCoupon(Long couponId, Long customerId, String email) Map<Long, List<Long>> map redeemCoupon(CustomerId customer, CouponId cupon, Email email) Micro-Types class CustomerId { private final Long id; ...get/hash/equals } @Value // Lombok class CouponId { Long id; } Fight the by crea4ng Primi;ve Obsession record Email(String value) { String domain() {..} } Type-safe seman;cs ➖ early decision ➖ surprising ➖ memory usage++ Fix: recycle in-memory instances: ProductId.of(1L) == ProductId.of(1L) (un8l Project Valhalla is done) Logic inside vs an U;l 👍 Use for core IDs Tuple3<Long, String, List<Long>> Map<CustomerId, List<OrderId>> orders Tuple3<CustomerId, Region, List<OrderId>> enum Region { EMEA, NA, LATAM, APAC } Restrict values
24. 90 VictorRentea.ro a training by
25. VictorRentea.ro 91 Functional Programming
26. VictorRentea.ro 92 Functional Programming Code Smells 1) Pass behavior around f(x → ...) f(new Consumer<X>() { void accept(X x) {...} }) = Func'ons are first-class ci'zens 2) Simplify work with collec=ons newList=list.stream().filter(→).map(→).toList(); + Less muta'on: add, remove.. In our daily life:
27. VictorRentea.ro 93 Functional Programming Func3ons should be ........... Data should be ................... No Side Effects Same input è Same output (adjec've) = Programming without side effects (adjec've) immutable pure Philosophy of
28. VictorRentea.ro 94
29. VictorRentea.ro 95 Long-Lived Mutable Data in multi-threaded code = 💀 race bugs Immutable💚 Objects Code Smells in complex flows = 💀 hard to track changes Fragmented Immutable if designed too large: Memory Churn if cloned repeatedly: Cumbersome if mixed with an ORM: https://gist.github.com/victorrentea/67f923dca33c7e737c867f60c727c604 list5000.fold(listOf<Int>()) { list, e -> list + e } immutable
30. VictorRentea.ro 96 Ugly large constructor x = new X(1, 2, 3, false, -1, null, "X"); L .toBuilder() allows unrestricted changes var obj2 = obj1.toBuilder().a(1).b(2)...build(); J Meaningful methods, that may guard rules builder.fullName("First", "Last")... // manual builder var obj2 = obj1.withFullName("First", "Last"); // wither J Deeper model (new types) var obj2 = obj1.withFullName(new FullName("F", "L")); Code Smells Fragmented Immutable Builder >> Named parameters x = X(a=1,b=2,c=3,...) data.copy(a=1, b=2)
31. VictorRentea.ro 97 double avg = 0; for (Employee e : employees) { avg += e.getSalary(); } log.debug("Average:" + avg); avg = avg / employees.size(); 1 2 3 4 5 6 sum Code Smells ≠ 0 sum Here avg means "sum" This code lies to you! Don't reassign local variables Recycled variable Confused Variable sum Mind IDE warnings Define a new var final const val IntelliJ can automa6cally add final everywhere possible: SO Error Prone fails compila6on unless it's annotated with @Var
32. VictorRentea.ro 98 Temporal Coupling the order of opera6ons maDers for mysterious reasons handle(Form form) { form = validate(form); form = normalize(form); ... form = process(form); audit(form); } handle(Form form) { var valid = validate(form); var normal = normalize(valid); ... var withId = process(normal); audit(withId); } Reordering lines produces bugs Code Smells Don't reassign local variables Recycled variable Confused Variable
33. VictorRentea.ro 100 Code Smells for
34. VictorRentea.ro 101 Code Smells Loop for 😮💨 Phew, I only use while Ø if it can be replaced with FP: .filter, .map… 2019 Ø if it does more than one thing
35. VictorRentea.ro 102 Loop Complex Loop for (e : list) { results.add(...) total += ... sideEffect(e); } var results = new ArrayList(); for (e : list) { results.add(...) } for (e : list) {total += ...} var total = list.stream()….sum()/reduce for (e : list) sideEffect(e); list.forEach(e -> sideEffect(e)); Split Loop var results = list.stream()….collect(… doing unrelated things Accumulator Loop gathering data via a loop Code Smells 🤔 ⚠ no performance hit for typical BE ⚠ order of side-effects can maker
36. VictorRentea.ro 104 Code Smells !// TODO: sum active orders int total = 0; orders.stream() .filter(order -> order.isActive()) .forEach(order -> { total += order.getPrice(); }); compila>on fails lambdas can't change variables on stack !// AtomicInteger total; total.incrementAndGet(price); !// int[] sum = {0}; 🤨 total[0] += price; this.total += price;!//field🚫 Hack move mutable state on heap avoidable side-effects in a FP pipe Mutant Pipeline int total = orders.stream() .filter(Order!::isActive) .mapToInt(Order!::getPrice) .sum();✅ Correct compute and return .forEach .reduce(0, Integer!::sum);!//avoid
37. VictorRentea.ro 105 avoidable side-effects in a FP pipe Code Smells stream.forEach(e → ...):void optional.ifPresent(e → ...):void .forEach ❌ Code smells when used to accumulate data: ❌ .forEach(e → map.put(e.id(), e));è.collect(toMap()):Map ❌ .forEach(e → adder.increment(e)); è.sum():int ❌ .ifPresent(e → list.add(e));è.flatMap(Optional::stream) ✅ OK to do external side effects: ✅ .forEach(m → mailSender.send(m)); è mailSender::send❤ ✅ .forEach(e → e.setStartedAt(now())); ✅ .ifPresent(e → repo.save(e)); è {save(it)} Kt❤ Mutant Pipeline
38. VictorRentea.ro 106 .peek(list::add) // also Avoid side-effects in .forEach (if possible) .map(e -> {list.add(e); return e;}) Meanwhile: Key Point 😮💨 (but oUen necessary) Side-effects are bad
39. VictorRentea.ro 107 Func%onal Programming Misuse > Abuse
40. VictorRentea.ro 108 List<Product> streamWreck(List<Order> orders) { return orders.stream() .filter(o -> o.getCreationDate().isAfter(now().minusYears(1))) .flatMap(o -> o.getOrderLines().stream()) .collect(groupingBy(OrderLine!::getProduct, summingInt(OrderLine!::getItemCount))) .entrySet() .stream() .filter(e -> e.getValue() !>= 10) .map(Entry!::getKey) .filter(p -> !p.isDeleted()) .filter(p -> !productRepo.findByHiddenTrue().contains(p)) .collect(toList()); } Code Smells Func<onal Chainsaw frequentProducts = orderedProducts() Order::isRecent è extract explanatory variables and func>ons aFer every ~ 3-4 operators Expression Func>ons (en6re body is an expression) fun x(…) = <expr> x = (…) => <expr>
41. VictorRentea.ro 109 Complex folding return list.reduce((prev, e) => // TypeScript new Dec(prev?.price || 0).gte(e.price || 0) ? prev : e, undefined); ✅ Prep the collec7on using .filter() .map() and keep reduce trivial const maxByPrice = (a, b) => new Dec(a.price).gte(b.price) ? a : b; return list.filter(({price}) => !!price).reduce(maxByPrice); Reduce Rodeo ✅ Use specialized collectors .sum() .toList() .max() .average() Use reduce for: a) unusual accumula7on b) compute mul7ple results eg max+min in a single pass c) performance (measured) d) JS/TS, kept simple⚠
42. VictorRentea.ro 110 @Bean public Function<Flux<LikeEvent>, Flux<LikedPosts!>> onLikeEvent() { return flux -> flux .doOnNext(event -> postLikes.put(event.postId(),event.likes())) .doOnNext(event -> eventSink.tryEmitNext(event)) .map(LikeEvent!::postId) .buffer(ofSeconds(1)) .flatMap(ids -> postRepo.findAllById(ids) .map(Post!::title) .collectList()) .map(LikedPosts!::new) .onErrorContinue((x,e) ->log.error(STR."Ignore \{x} for \{e}")) .doOnNext(message -> log.info("Sending: " + message)); } ☠ Reac%ve Programming ☠ Chaining is mandatory Reac=ve Programming is NOT clean!* Goodbye variables and excep4ons. And learn 100+ operators. * there are some safe coding best pracIces, that I teach in my ReacIve Programming workshop.
43. VictorRentea.ro 111 Returning technical errors Try<Long> justTrying(Data data) { // not FP Try<String> inputTry = process(data); if (inputTry.isFailure()) { return Try.failure(inputTry.getCause()); } log.info("X"); Try<Long> idTry = inputTry.flatMap(this::save); if (idTry.isSuccess()) { audit(data.input()); } return idTry; } Double-Edged Return Try<String> process(Data data) { if (data.name().isBlank()) return Try.failure(new AnException("…")); ... return Try.success(result); } Just like checked excep6ons(🤮) throws AnException Try<Long> tryingFP(Data data) { return process(data) .onSuccess(e -> log.info("X")) .flatMap(repo::save) .andThen(id -> audit(data)); } // feeling zmart? 😎🤓 Long boring(Data data) { String r = process(data); log.info("X"); saveToDb(r); audit(data); } Pollutes caller throw new ... Use it to: - Return a sealed type carrying the business outcome of an ac6on - Collect all failed items in a list Java: vavr lib Kotlin: Result Caught in transla4on ... return result; }
44. VictorRentea.ro 112 Returning technical errors Double-Edged Return Try<Long> tryingFP(Data data) { return process(data) // 😱 .onSuccess(e -> log.info("X")) .flatMap(repo::save) .andThen(id -> audit(data)); } Use it to: - Return a sealed type carrying the business outcome of an ac6on - Collect all failed items in a list Long boring(Data data) { String r = process(data); log.info("X"); saveToDb(r); audit(data); } String process(Data data) { if (data.name().isBlank()) throw new Ex("…"); ... return result; } Try<String> process(Data data) { if (data.name().isBlank()) return Try.failure(new Ex("…")); ... return Try.success(result); } throws Checked Caught in Transi7on nostalgic🥺 for Go,Scala,Kotlin,JS In FP don't throw pollutes caller
45. VictorRentea.ro 113 Code Smells <T> Consumer<T> uncheck(ThrowingConsumer<T> f) { return x -> { try { f.accept(x); } catch (Throwable e) { throw new RuntimeException(e); } }; // what does this do ? } .forEach(uncheck(fileWriter::write)); // caller avoid taking / returning behavior whenever possible Func<on Factory Frenzy
46. VictorRentea.ro 115 Dead ☠ Code
47. VictorRentea.ro 116 Code Smells Dead ☠ Code Not Referenced A param, variable, method, unused in IDE è Delete [with IDE] 🥳 Unreachable Code How to find code never called in produc4on? èMonitor API calls in prod ègit blame: if recent🤞 > Ask author now! (6 month later: It works? Don't touch it! ™ ) è log.warn("🙏Delete after Sep'24 if this is in not in log of the last 6 months*") if (impossible) { lots of code* lots of code } ... you think 😬
48. 118 VictorRentea.ro a training by
49. VictorRentea.ro 120 Ever wrote code an=cipa=ng a future requirement, or a hoping a wider use (eg. a shared library)? I need a bike! - biz
50. VictorRentea.ro 121 Overengineering aka Specula4ve Generality Keep It Short & Simple (KISS) Code Smells Yes! - a bright developer Ever wrote code an+cipa+ng a future requirement, or a hoping a wider use (eg. a shared library)? But it's fun!! Pet-project! Nothing is more difficult than finding a simple solu4on to a complex problem. Simplicity is the ul=mate sophis=ca=on. -- Leonardo DaVinci don't buildwalls in front of probable change,
51. 124 VictorRentea.ro a training by
52. 125 VictorRentea.ro a training by
53. 126 VictorRentea.ro a training by
54. VictorRentea.ro 127 Java Weaknesses Op6onal<> @Nullable null ❤ Run6me Excep6ons Checked ExcepJons Code Smells Java got its name by people that had no idea if they were awake or asleep due to coffee abuse. records [17] Lombok Boilerplate Code geDer, seDer, hashCode/equals, toString No Extension FuncJons U6ls No Default / Named Params Overloading Builders ImmutableList (Guava) unmodifiableList(list) List.of() [11] .toList() [17] Mutable CollecJons list.add(e)
55. VictorRentea.ro 129 Code Smells most common as of 2024 Repeated Switches Monster Method God Class Many Parameters Heavy Lambda Flags OOP Middle Man Data Classes Missing Abstrac7on Primi7ve Obsession Feature Envy Overengineering Dead Code Impera7ve FP Complex Loop Accumulators FP Chainsaw Mutable Data Temporary Field Confused Variable FP Bad Names Reduce Rodeo
56. VictorRentea.ro 130 Stay into The Light sourcemaking.com/refactoring Chap. 1-11 + 17 Chapter 2+3 +links For newbies Coding Katas kata-log.rocks/refactoring cleancoders.com refactoring.guru Summary ar4cle: link ↑ Seed Passion for Clean Code Clean Code - Reading Guide A 4me-efficient way to get up to speed with Clean Code and Refactoring 2008 2018
57. VictorRentea.ro 131 The End is this
58. European So=ware Cra=ers hDps://www.meetup.com/european-soFware-craFers The largest community in the world on - craUing good quality code, - simple design - robust unit tests - con'nuous growth & teamwork Mee'ng every month a7er hours (1700 CET) online on Zoom + YouTube Live Free Forever! I was VictorRentea.ro