Slide 1

Slide 1 text

VictorRentea.ro 49 Code Smells by Victor Rentea

Slide 2

Slide 2 text

👋 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

Slide 3

Slide 3 text

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

Slide 4

Slide 4 text

VictorRentea.ro 52 Code Smells code paAerns that hurt you later “If it stinks, you have to change it.”

Slide 5

Slide 5 text

VictorRentea.ro 53 Code Smells Copy-Paste Programming DRY Don't Repeat Yourself

Slide 6

Slide 6 text

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 lions

Slide 7

Slide 7 text

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

Slide 8

Slide 8 text

VictorRentea.ro 57 Code Smells Bad Names

Slide 9

Slide 9 text

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

Slide 10

Slide 10 text

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

Slide 11

Slide 11 text

61 VictorRentea.ro a training by Con2nuous Dis2la2on Rename

Slide 12

Slide 12 text

VictorRentea.ro 63 Stuff That Starts Small... Code Smells ß 5 lines of code 😇 then 7, 10, 15, 25, 45...

Slide 13

Slide 13 text

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

Slide 14

Slide 14 text

71 VictorRentea.ro a training by When in Doubt, Extract a Method

Slide 15

Slide 15 text

VictorRentea.ro 75 Code Smells related data elements moving around together Missing Type String int int String

Slide 16

Slide 16 text

VictorRentea.ro 76 Map> è List Data Clumps Create more classes to group data that s4cks together int, int è Interval Tuple4 è 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

Slide 17

Slide 17 text

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

Slide 18

Slide 18 text

78 VictorRentea.ro a training by Code Smells

Slide 19

Slide 19 text

VictorRentea.ro 80 Code Smells class state (fields) logicLogic using the state of another object Feature Envy

Slide 20

Slide 20 text

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);

Slide 21

Slide 21 text

VictorRentea.ro 86 public Customer findById(id) { return repo.findById(id); } int startYear() { return yearInterval.start(); } Middle Man Indirec4on without abstrac4on Code Smells NOISE

Slide 22

Slide 22 text

VictorRentea.ro 87 confused by raw data without meaning Map> map Map> 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> What do these mean? 😨 one of: EMEA|NA|LATA|APAC

Slide 23

Slide 23 text

88 VictorRentea.ro a training by void redeemCoupon(Long couponId, Long customerId, String email) Map> 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> Map> orders Tuple3> enum Region { EMEA, NA, LATAM, APAC } Restrict values

Slide 24

Slide 24 text

90 VictorRentea.ro a training by

Slide 25

Slide 25 text

VictorRentea.ro 91 Functional Programming

Slide 26

Slide 26 text

VictorRentea.ro 92 Functional Programming Code Smells 1) Pass behavior around f(x → ...) f(new Consumer() { 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:

Slide 27

Slide 27 text

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

Slide 28

Slide 28 text

VictorRentea.ro 94

Slide 29

Slide 29 text

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()) { list, e -> list + e } immutable

Slide 30

Slide 30 text

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)

Slide 31

Slide 31 text

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

Slide 32

Slide 32 text

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

Slide 33

Slide 33 text

VictorRentea.ro 100 Code Smells for

Slide 34

Slide 34 text

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

Slide 35

Slide 35 text

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

Slide 36

Slide 36 text

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

Slide 37

Slide 37 text

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

Slide 38

Slide 38 text

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

Slide 39

Slide 39 text

VictorRentea.ro 107 Func%onal Programming Misuse > Abuse

Slide 40

Slide 40 text

VictorRentea.ro 108 List streamWreck(List 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 Funcons aFer every ~ 3-4 operators Expression Func>ons (en6re body is an expression) fun x(…) = x = (…) =>

Slide 41

Slide 41 text

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⚠

Slide 42

Slide 42 text

VictorRentea.ro 110 @Bean public Function, Flux> 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.

Slide 43

Slide 43 text

VictorRentea.ro 111 Returning technical errors Try justTrying(Data data) { // not FP Try inputTry = process(data); if (inputTry.isFailure()) { return Try.failure(inputTry.getCause()); } log.info("X"); Try idTry = inputTry.flatMap(this::save); if (idTry.isSuccess()) { audit(data.input()); } return idTry; } Double-Edged Return Try process(Data data) { if (data.name().isBlank()) return Try.failure(new AnException("…")); ... return Try.success(result); } Just like checked excep6ons(🤮) throws AnException Try 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; }

Slide 44

Slide 44 text

VictorRentea.ro 112 Returning technical errors Double-Edged Return Try 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 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

Slide 45

Slide 45 text

VictorRentea.ro 113 Code Smells Consumer uncheck(ThrowingConsumer 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

Slide 46

Slide 46 text

VictorRentea.ro 115 Dead ☠ Code

Slide 47

Slide 47 text

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 😬

Slide 48

Slide 48 text

118 VictorRentea.ro a training by

Slide 49

Slide 49 text

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

Slide 50

Slide 50 text

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,

Slide 51

Slide 51 text

124 VictorRentea.ro a training by

Slide 52

Slide 52 text

125 VictorRentea.ro a training by

Slide 53

Slide 53 text

126 VictorRentea.ro a training by

Slide 54

Slide 54 text

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)

Slide 55

Slide 55 text

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

Slide 56

Slide 56 text

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

Slide 57

Slide 57 text

VictorRentea.ro 131 The End is this

Slide 58

Slide 58 text

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