Slide 1

Slide 1 text

Thai Pangsakulyanont 1 Smells in React Apps JSConf.Asia 2018 (with transcript added) Note: The transcript is extracted and edited from the recorded talk.

Slide 2

Slide 2 text

Slide № 2 This is a blank slide.

Slide 3

Slide 3 text

Slide № 3 Sawasdee krub (Hello in Thai) This is my first time giving a full-length talk at a conference outside Thailand. I’m very grateful to the organizers for having invited me here.

Slide 4

Slide 4 text

Slide № 4 Smells in React Apps Today, I will talk about

Slide 5

Slide 5 text

Slide № 5 Refactoring: Improving the Design of Existing Code Clean Code: A Handbook of Agile Software Craftsmanship Agile Software Development, Principles, Patterns, and Practices There are many books that talk about code smell: Unfortunately, I don’t read many books. Usually, I read articles on the internet and piece the information together. So, these 3 books, I haven’t read them yet.

Slide 6

Slide 6 text

Slide № 6 (Contents afterwards are mainly based on my experience.) They may not perfectly align with those books. I may have used a different term. I really hope they will be useful to you. Therefore…

Slide 7

Slide 7 text

About Me Slide № 7 Thai Pangsakulyanont (@dtinth) Please tweet me your feedback. (from Thailand)

Slide 8

Slide 8 text

Slide № 8 Find me on GitHub:

Slide 9

Slide 9 text

Slide № 9 Loves to build software for fun. In my free time, I like to work on side projects.

Slide 10

Slide 10 text

Slide № 10 A web-based musical instrument with several instruments to choose from. I demoed it at JSConf.Asia 2016, in a lightning talk. dtinth/midi-instruments https://302.dt.in.th/midi-instruments-pv

Slide 11

Slide 11 text

Slide № 11 dtinth/atom-aesthetic-ui Some other time, I feel like doing something a bit silly. So, I created a Windows 2000 theme for Atom text editor.

Slide 12

Slide 12 text

Slide № 12 dtinth/atom-aesthetic-ui Main challenge: Replicate the Windows Explorer tree view using CSS. That was really fun!

Slide 13

Slide 13 text

Slide № 13 dtinth/GyroscratchAndroid bemusic/gyroscratch-web The other time, I want to play a music game, like a DJ. I need a turntable. So I put my phone on a dish, and make it transmit gyroscope data to the game. https://302.dt.in.th/gyroscratch-pv

Slide 14

Slide 14 text

Slide № 14 Loves to build software for fun. For me the “fun” part is very important. It makes coding a fulfilling thing to do. It can also improve my professional coding skills. So there you see it.

Slide 15

Slide 15 text

Slide № 15 Interested in the ways
 to make software maintainable. I am also

Slide 16

Slide 16 text

Slide № 16 One of my old project… (2009) That’s because… I put in a lot of effort developing it. At that time, I didn’t learn much about software engineering, so I keep on coding and coding. Over time, the project become less maintainable.

Slide 17

Slide 17 text

Slide № 17 7000 lines of untestable code. There we have… At that point, when I do something or fix something, more bugs appear.

Slide 18

Slide 18 text

Slide № 18 7000 lines of untestable code. Maintain: Not fun Later, I learn about software engineering. I feel that I want to improve the codebase, add tests, etc.

Slide 19

Slide 19 text

Slide № 19 7000 lines of untestable code. Maintain: Not fun Improve the codebase: Not fun Because I have… It is a huge boring task! A side project should be fun, not a burden. I start to feel that I’d rather do something else more fun.

Slide 20

Slide 20 text

Slide № 20 Development stopped So…

Slide 21

Slide 21 text

Slide № 21 Development stopped :( That’s sad, because I spent years working it.

Slide 22

Slide 22 text

Slide № 22 What I learned…

Slide 23

Slide 23 text

Slide № 23 Sustainability matters! When I work on a new project, I need to make sure it will not end up like that project again.

Slide 24

Slide 24 text

Slide № 24 Interested in the ways
 to make software maintainable. That’s why I’m interested in these stuff:

Slide 25

Slide 25 text

Slide № 25 Object-oriented analysis and design Clean architecture Functional programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … There are many contributing factors. Many techniques you can use to make software more maintainable.

Slide 26

Slide 26 text

Slide № 26 Later… (2014)

Slide 27

Slide 27 text

Slide № 27 bemusic/bemuse bemusic/bemuse https://302.dt.in.th/bemuse-pv ADVERTISEMENT https://bemuse.ninja/ I started working on a new project, called Bemuse. It’s a web-based, open-source music game built using HTML5 technologies. I want to make it a great game and I want people to play it. So I need to make sure this project is maintainable.

Slide 28

Slide 28 text

Slide № 28 I organize my code into modules…

Slide 29

Slide 29 text

Slide № 29 …and use to bundle them together.

Slide 30

Slide 30 text

Slide № 30 Static analysis with ESLint I set up ESLint to catch common errors…

Slide 31

Slide 31 text

Slide № 31 Static analysis with ESLint …and enforce a consistent coding style.

Slide 32

Slide 32 text

Slide № 32 Unit testing (Mocha)

Slide 33

Slide 33 text

Slide № 33 Continuous integration & deployment (CircleCI)

Slide 34

Slide 34 text

Slide № 34 Code coverage (Codecov)

Slide 35

Slide 35 text

Slide № 35 3 weeks of project setup It took me No game development during that time. Sounds pretty long compared to other projects.

Slide 36

Slide 36 text

Slide № 36 Result It made a huge impact.

Slide 37

Slide 37 text

Slide № 37 :) Unit tests increases confidence when making a huge change to the codebase. They helped me confidently switch to use another library, without the fear that I might break the game

Slide 38

Slide 38 text

Slide № 38 Decoupled architecture It allowed me to try out different view libraries (2 times) without having to rewrite the whole app.

Slide 39

Slide 39 text

Slide № 39 Implementation details behind an interface It allowed me to switch to a new backend service without having to make a sweeping change across the whole codebase

Slide 40

Slide 40 text

Slide № 40 Some features can be prototyped in 1 night

Slide 41

Slide 41 text

Slide № 41 A good codebase is like a clean table. I learned that… In a good codebase, I can experiment quickly and prototype new ideas right away…

Slide 42

Slide 42 text

Slide № 42 …without having to first move so many stuffs that’s getting in my way! After the experiment is finished…

Slide 43

Slide 43 text

Slide № 43 …I try to refactor the codebase to be clean again, …ready to take in more experiments to come in the future.

Slide 44

Slide 44 text

Slide № 44 I’ve been maintaining this game for 3 years, and I’m still maintaining it to this day.

Slide 45

Slide 45 text

Slide № 45 Present (2018)

Slide 46

Slide 46 text

Slide № 46 Front-end architect at @taskworld * Opinion expressed are my own.

Slide 47

Slide 47 text

Slide № 47 React + Redux Our app is built using

Slide 48

Slide 48 text

Slide № 48 We have many features. This is the tree view of our Redux store.

Slide 49

Slide 49 text

Slide № 49 Redux store

Slide 50

Slide 50 text

Slide № 50 Redux store React components

Slide 51

Slide 51 text

Slide № 51 Redux store React components 900+ components

Slide 52

Slide 52 text

Slide № 52 Redux store React components 900+ components Over 2 years

Slide 53

Slide 53 text

Slide № 53 Redux store React components 900+ components Over 2 years, 20+ engineers worked on it It is a challenge for us to keep innovating while making sure the code is maintainable. Have to strike a healthy balance: minimizing tech debt vs shortening time to market

Slide 54

Slide 54 text

Slide № 54 Object-oriented analysis and design Clean architecture Functional programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … As I said, there are many contributing factors:

Slide 55

Slide 55 text

Slide № 55 Object-oriented analysis and design Clean architecture Functional programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … Tech stuffs The things that I listed here are…

Slide 56

Slide 56 text

Slide № 56 Communication But when working in a team, I learned that the most important thing really is

Slide 57

Slide 57 text

Slide № 57 Example

Slide 58

Slide 58 text

Slide № 58 // MusicPreviewer.js export function preload () { getInstance() } export function enable () { return getInstance().enable() } export function disable () { return getInstance().disable() } export function go () { return getInstance().go() } export function preview (url) { return getInstance().preview(url) } MusicPreviewer

Slide 59

Slide 59 text

Slide № 59 // MusicPreviewer.js export function preload () { getInstance() } export function enable () { return getInstance().enable() } export function disable () { return getInstance().disable() } export function go () { return getInstance().go() } export function preview (url) { return getInstance().preview(url) } MusicPreviewer exports 5 functions

Slide 60

Slide 60 text

Slide № 60 // MusicPreviewer.js export function preload () { getInstance() } export function enable () { return getInstance().enable() } export function disable () { return getInstance().disable() } export function go () { return getInstance().go() } export function preview (url) { return getInstance().preview(url) } import * as MusicPreviewer from MusicPreviewer.preload() MusicPreviewer.enable() MusicPreviewer.disable() MusicPreviewer.go() MusicPreviewer.preview(url) User code: The rest of the app only needs to know about these 5 functions

Slide 61

Slide 61 text

Slide № 61 Behind the interface They don’t need to know that… …lies a big, ugly hack.

Slide 62

Slide 62 text

Slide № 62 let instance const getInstance = () => instance || ( instance = createMusicPreviewer() ) function createMusicPreviewer () { let enabled = false let currentUrl = null let backgroundLoaded = false let backgroundPlayed = false const instances = {} function update () { if (!enabled) { if (backgroundPlayed) { backgroundFader.fadeTo(0, 100) backgroundPlayed = false background.pause() } for (const key of Object.keys(instances)) { const instance = instances[key] instance.destroy() } return } let playing = null for (const key of Object.keys(instances)) { const instance = instances[key] if (key === currentUrl) { if (instance.loaded) { instance.play() playing = instance } else { instance.stop() } } if (playing) { backgroundFader.fadeTo(0, 1) } else { backgroundFader.fadeTo(0.4, 0.5) if (backgroundLoaded && !backgroundPlayed) { backgroundPlayed = true try { background.play() } catch (e) { console.warn('Cannot play background music') } } } } const musicPreviewer = { enable () { if (enabled) return enabled = true update() }, disable () { if (!enabled) return enabled = false update() }, go () { if (!enabled) return goSound.currentTime = 0

Slide 63

Slide 63 text

Slide № 63 Wrote a hack behind a well-designed interface In principle: Bad code stays behind that interface. This would work fine. Other parts of software not affected by this.

Slide 64

Slide 64 text

Slide № 64 Wrote a hack behind a well-designed interface In principle: Bad code stays behind that interface. In practice: Another person looks for a technical solution,

Slide 65

Slide 65 text

Slide № 65 Wrote a hack behind a well-designed interface In principle: Bad code stays behind that interface. In practice: Another person looks for a technical solution, found my hack (but didn’t know it was a hack), because I didn’t communicate clearly

Slide 66

Slide 66 text

Slide № 66 Wrote a hack behind a well-designed interface In principle: Bad code stays behind that interface. In practice: Another person looks for a technical solution, found my hack (but didn’t know it was a hack), and duplicated the code! If it’s a hack/non-exemplar code, then it’s really important to communicate why the code was written that way.

Slide 67

Slide 67 text

Slide № 67 Effective code reviews Another important factor is having…

Slide 68

Slide 68 text

Slide № 68 Smells To be effective, in my opinion, we should learn to recognize the… …inside our code.

Slide 69

Slide 69 text

Slide № 69 Information leaks The first kind of smell, I call it:

Slide 70

Slide 70 text

Slide № 70 Example

Slide 71

Slide 71 text

Slide № 71 Standard UI kit In our project, we have: …which contains the common components used across our app

Slide 72

Slide 72 text

Slide № 72 A UserAvatar component Can be used to render user’s avatar by specifying the image and size.

Slide 73

Slide 73 text

Slide № 73 A UserAvatar component

Slide 74

Slide 74 text

Slide № 74 A UserAvatar component

Slide 75

Slide 75 text

Slide № 75 A UserAvatar component (Oops, that’s a bit too large…)

Slide 76

Slide 76 text

Slide № 76 A UserAvatar component

Slide 77

Slide 77 text

Slide № 77 New feature! Messaging system The code changes over time. Later, as we develop, we need to implement new features.

Slide 78

Slide 78 text

Slide № 78 Thai Pangsakulyanont 1 minute ago Let’s go to the Marina Bay Carnival after the conference!!!! UserAvatar on the left, text on the right… This should be straightforward. This is how a message should look like.

Slide 79

Slide 79 text

Slide № 79 So, I look at the UserAvatar component…

Slide 80

Slide 80 text

Slide № 80 A UserAvatar component

Slide 81

Slide 81 text

Slide № 81 A UserAvatar component the sized that we need to use is not there!

Slide 82

Slide 82 text

Slide № 82 A UserAvatar component To fix this, we need to add a new size. But we already have small, medium, and large. What should we put in here?

Slide 83

Slide 83 text

Slide № 83 A UserAvatar component We did the most obvious thing: Does this smell a bit to you? Let’s keep going on!

Slide 84

Slide 84 text

Slide № 84 Few months later

Slide 85

Slide 85 text

Slide № 85 Few months later everywhere! …in places totally unrelated to the message

Slide 86

Slide 86 text

Slide № 86 Step out of code Look at the big picture

Slide 87

Slide 87 text

Slide № 87 UserAvatar Message

Slide 88

Slide 88 text

Slide № 88 UserAvatar specific feature Message

Slide 89

Slide 89 text

Slide № 89 UserAvatar specific feature generic component Message

Slide 90

Slide 90 text

Slide № 90 UserAvatar specific feature generic component I need to show an avatar! Message

Slide 91

Slide 91 text

Slide № 91 UserAvatar uses specific feature generic component Message I need to show an avatar!

Slide 92

Slide 92 text

Slide № 92 UserAvatar uses specific feature generic component I know how big to display in a message Message

Slide 93

Slide 93 text

Slide № 93 UserAvatar uses aware of specific feature generic component Message I know how big to display in a message

Slide 94

Slide 94 text

Slide № 94 UserAvatar uses aware of specific feature generic component Information leaking. Message

Slide 95

Slide 95 text

Slide № 95 UserAvatar Message uses aware of specific feature generic component Information leaking. Knows how to render itself Except: Doesn’t know the appropriate size of the avatar

Slide 96

Slide 96 text

Slide № 96 UserAvatar Message uses aware of specific feature generic component Information leaking. Knows how to render itself Except: Doesn’t know the appropriate size of the avatar The knowledge of the size, which should have belonged to the messaging system is here That knowledge leaked into the UserAvatar component.

Slide 97

Slide 97 text

Slide № 97 Leakage Information about a specific feature leaked into a generic component …making the component less generic, more awkward to reuse

Slide 98

Slide 98 text

Slide № 98 Generic component not flexible enough. Why did this happen? Maybe…

Slide 99

Slide 99 text

Slide № 99 Make the generic component more flexible Fix:

Slide 100

Slide 100 text

Slide № 100 A UserAvatar component We used to have 4 sizes:

Slide 101

Slide 101 text

Slide № 101 A UserAvatar component XS S M L XL XXL XXXL Now we have 8 sizes:

Slide 102

Slide 102 text

Slide № 102 UserAvatar specific feature generic component I need to show an avatar! It’s going to have size M! Message Now Message can pick a size from the catalog.

Slide 103

Slide 103 text

Slide № 103 A UserAvatar component function UserAvatar (props) { … } UserAvatar.propTypes = { size: PropTypes.oneOf([ 'XS', 'S', 'M', 'L', 'XL', 'XXL', 'XX … } Pitfall: What would happen if we want to make a new size between L and XL? We may need to adjust the size table and that means a sweeping change across the codebase to shift sizes. Fortunately this is flexible enough at Taskworld.

Slide 104

Slide 104 text

Slide № 104 A UserAvatar component function UserAvatar (props) { … } UserAvatar.propTypes = { size: PropTypes.oneOf([ 'XS', 'S', 'M', 'L', 'XL', 'XXL', 'XX … } If you want to make it really flexible…

Slide 105

Slide 105 text

Slide № 105 A UserAvatar component function UserAvatar (props) { … } UserAvatar.propTypes = { size: PropTypes.number, … } Let the users specify the size in pixel units. But it may go out of control.

Slide 106

Slide 106 text

Slide № 106 A UserAvatar component function UserAvatar (props) { … } UserAvatar.propTypes = { size: PropTypes.oneOf([ 18, 24, 30, 36, 48, … ]), … } To keep things under control, you have a list of allowed sizes. There are trade-offs to all these approaches. This is an example where a smell calls for a bit of restructuring…

Slide 107

Slide 107 text

Slide № 107 Sometimes it’s just naming. But… All you have to do is just changing the name!

Slide 108

Slide 108 text

Slide № 108 Example

Slide 109

Slide 109 text

Slide № 109 Standard UI kit

Slide 110

Slide 110 text

Slide № 110 Upload file

Slide 111

Slide 111 text

Slide № 111 Upload file The user can click on a button.

Slide 112

Slide 112 text

Slide № 112 Upload file

Slide 113

Slide 113 text

Slide № 113 Upload file Select file on computer… Dropbox Google Drive Emergency alert

Slide 114

Slide 114 text

Slide № 114 Upload file Select file on computer… Dropbox Google Drive Emergency alert Select file on computer… The user can select an item from a menu.

Slide 115

Slide 115 text

Slide № 115 Upload file Select file on computer… Dropbox Google Drive Emergency alert I have a

Slide 116

Slide 116 text

Slide № 116 Upload file Select file on computer… Dropbox Google Drive Emergency alert I have a

Slide 117

Slide 117 text

Slide № 117 Select file on computer… Dropbox Google Drive Emergency alert Upload file Ah!!!

Slide 118

Slide 118 text

Slide № 118 Select file on computer… Dropbox Google Drive Emergency alert Upload file !

Slide 119

Slide 119 text

Slide № 119 Upload file

Slide 120

Slide 120 text

Slide № 120 Upload file When you click on a MenuButton…

Slide 121

Slide 121 text

Slide № 121 Select file on computer… Dropbox Google Drive Emergency alert Upload file …a menu opens…

Slide 122

Slide 122 text

Slide № 122 Select file on computer… Dropbox Google Drive Emergency alert Upload file Select file on computer… …and you can select items from it.

Slide 123

Slide 123 text

Slide № 123 Select file on computer… Dropbox Google Drive Emergency alert Upload file Select file on computer… Button must appear to be pressed while menu is open The catch:

Slide 124

Slide 124 text

Slide № 124 Upload file Only when the menu is closed, then the button can return to its normal state.

Slide 125

Slide 125 text

Slide № 125 Upload file Button.propTypes = { children: PropTypes.node } At first thought, we may start with the most obvious thing here…

Slide 126

Slide 126 text

Slide № 126 Upload file Button.propTypes = { children: PropTypes.node, isMenuOpen: PropTypes.bool }

Slide 127

Slide 127 text

Slide № 127 Button.propTypes = { children: PropTypes.node, isMenuOpen: PropTypes.bool } Upload file If true, then the button appears to be pressed. Again… Does it smell a bit?

Slide 128

Slide 128 text

Slide № 128 Button.propTypes = { children: PropTypes.node, isMenuOpen: PropTypes.bool } Button is now aware of the menu Upload file An entirely unrelated concept!

Slide 129

Slide 129 text

Slide № 129 Button.propTypes = { children: PropTypes.node, isMenuOpen: PropTypes.bool } Upload file Thinking a bit deeper, we see that a doesn’t really need to know about the menu.

Slide 130

Slide 130 text

Slide № 130 Button.propTypes = { children: PropTypes.node, isDepressed: PropTypes.bool } Upload file It only needs to know whether it should appear to be…depressed. A simple naming change can entirely communicate a different thing!

Slide 131

Slide 131 text

Slide № 131 When reviewing code: “Does this really needs to know about that?”

Slide 132

Slide 132 text

Slide № 132 Low cohesion The second kind of smell, I call it:

Slide 133

Slide 133 text

Slide № 133 “Cohesion” Related things stay together.

Slide 134

Slide 134 text

Slide № 134 Example

Slide 135

Slide 135 text

Slide № 135 A navigation bar Now, for our app, we need to create…

Slide 136

Slide 136 text

Slide № 136 A navigation bar

Slide 137

Slide 137 text

Slide № 137 class Navigation extends Component { render () { } Using react-bootstrap, this should be straightforward.

Slide 138

Slide 138 text

Slide № 138 class Navigation extends Component { render () { return ( ) }

Slide 139

Slide 139 text

Slide № 139 class Navigation extends Component { render () { return ( My app ) }

Slide 140

Slide 140 text

Slide № 140 class Navigation extends Component { render () { return ( My app Home Explore Search ) }

Slide 141

Slide 141 text

Slide № 141 class Navigation extends Component { render () { return ( My app Home Explore Search ) } So far, so good!

Slide 142

Slide 142 text

Slide № 142 New feature!

Slide 143

Slide 143 text

Slide № 143 New feature! Authentication system We need to update the navbar…

Slide 144

Slide 144 text

Slide № 144 When logged out When logged in

Slide 145

Slide 145 text

Slide № 145 When logged out When logged in

Slide 146

Slide 146 text

Slide № 146 class Navigation extends Component { render () { return ( My app Home Explore Search ) } }

Slide 147

Slide 147 text

Slide № 147 class Navigation extends Component { 
 render () { return ( My app Home Explore Search

Slide 148

Slide 148 text

Slide № 148 class Navigation extends Component {
 static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { return ( My app Home Explore Search

Slide 149

Slide 149 text

Slide № 149 class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search

Slide 150

Slide 150 text

Slide № 150 const loggedIn = !!this.props.currentUser return ( My app Home Explore Search ) } }

Slide 151

Slide 151 text

Slide № 151 const loggedIn = !!this.props.currentUser return ( My app Home Explore Search ) } }

Slide 152

Slide 152 text

Slide № 152 const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {!loggedIn && LoginLogout ) } }

Slide 153

Slide 153 text

Slide № 153 class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Now, Navigation is just a presentational component. We need to connect to the Redux store.

Slide 154

Slide 154 text

Slide № 154 const enhance = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return (

Slide 155

Slide 155 text

Slide № 155 const enhance = connect( state => ({ currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Now, looking at the component, we can see that cleanly separates into 3 concerns…

Slide 156

Slide 156 text

Slide № 156 const enhance = connect( state => ({ currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Selecting data from store

Slide 157

Slide 157 text

Slide № 157 const enhance = connect( state => ({ currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Selecting data from store Dispatching action to store

Slide 158

Slide 158 text

Slide № 158 const enhance = connect( state => ({ currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Selecting data from store Dispatching action to store Rendering logic

Slide 159

Slide 159 text

Slide № 159 New features!

Slide 160

Slide 160 text

Slide № 160 Notification system Messaging system

Slide 161

Slide 161 text

Slide № 161 When logged out When logged in

Slide 162

Slide 162 text

Slide № 162 When logged out When logged in

Slide 163

Slide 163 text

Slide № 163 const enhance = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Coming back to our code…

Slide 164

Slide 164 text

Slide № 164 const enhance = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Selecting data from store Dispatching action to store Rendering logic We already see the pattern!

Slide 165

Slide 165 text

Slide № 165 const enhance = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( Selecting data from store Dispatching action to store Rendering logic Go with the flow!

Slide 166

Slide 166 text

Slide № 166 const enhance = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return (

Slide 167

Slide 167 text

Slide № 167 const enhance = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser

Slide 168

Slide 168 text

Slide № 168 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser

Slide 169

Slide 169 text

Slide № 169 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser

Slide 170

Slide 170 text

Slide № 170 onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore

Slide 171

Slide 171 text

Slide № 171 onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app

Slide 172

Slide 172 text

Slide № 172 onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app

Slide 173

Slide 173 text

Slide № 173 My app Home Explore Search {!loggedIn && LoginLogout ) } }

Slide 174

Slide 174 text

Slide № 174 My app Home Explore Search

Slide 175

Slide 175 text

Slide № 175 My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) }

Slide 176

Slide 176 text

Slide № 176 My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) } {loggedIn && Messages ({this.props.newMessageCount}) }

Slide 177

Slide 177 text

Slide № 177 My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) } {loggedIn && Messages ({this.props.newMessageCount}) }

Slide 178

Slide 178 text

Slide № 178 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) Let’s zoom out a bit…

Slide 179

Slide 179 text

Slide № 179 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) Selecting data from store Dispatching action to store Rendering logic Our code is still cleanly separated into 3 concerns!

Slide 180

Slide 180 text

Slide № 180 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) Selecting data from store Dispatching action to store Rendering logic Cross-cutting concerns Cross-cutting concerns have to do with many different functionalities in our app.

Slide 181

Slide 181 text

Slide № 181 User Notification Message User Message User Notification Message User Navigation Navigation User Navigation const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) But if we look from a functionality perspective… Many functional parts are intermixed inside the code: Hard to navigate.

Slide 182

Slide 182 text

Slide № 182 User Notification Message User Message User Notification Message User Navigation Navigation User Navigation const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) If we want to look at the code related to navigation we need to skip over a lot of unrelated code.

Slide 183

Slide 183 text

Slide № 183 Real-world example

Slide 184

Slide 184 text

Slide № 184 Bemuse options panel

Slide 185

Slide 185 text

Slide № 185 Bemuse options panel

Slide 186

Slide 186 text

Slide № 186 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644 --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return
You can also change the speed in-game
using the Up and Down arrow keys.
Speed will be automatically adjusted
to maintain a consistent note velocity.
The amount of play area to hide from the top.
Enable background animations (720p, alpha) Maintain absolute note velocity (advanced)
Save & Exit
} }) To do this, first, I have to add the code…

Slide 187

Slide 187 text

Slide № 187 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644 --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return
You can also change the speed in-game
using the Up and Down arrow keys.
Speed will be automatically adjusted
to maintain a consistent note velocity.
The amount of play area to hide from the top.
Enable background animations (720p, alpha) Maintain absolute note velocity (advanced)
Save & Exit
} }) Here!

Slide 188

Slide 188 text

Slide № 188 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644 --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return
You can also change the speed in-game
using the Up and Down arrow keys.
Speed will be automatically adjusted
to maintain a consistent note velocity.
The amount of play area to hide from the top.
Enable background animations (720p, alpha) Maintain absolute note velocity (advanced)
Save & Exit
} }) Here!

Slide 189

Slide 189 text

Slide № 189 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644 --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return
You can also change the speed in-game
using the Up and Down arrow keys.
Speed will be automatically adjusted
to maintain a consistent note velocity.
The amount of play area to hide from the top.
Enable background animations (720p, alpha) Maintain absolute note velocity (advanced)
Save & Exit
} }) Skipping 80 lines, and…

Slide 190

Slide 190 text

Slide № 190 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644 --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return
You can also change the speed in-game
using the Up and Down arrow keys.
Speed will be automatically adjusted
to maintain a consistent note velocity.
The amount of play area to hide from the top.
Enable background animations (720p, alpha) Maintain absolute note velocity (advanced) + + + Show expert gauge (experimental) + + +
Save & Exit
} }) Here!

Slide 191

Slide 191 text

Slide № 191 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644 --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return
You can also change the speed in-game
using the Up and Down arrow keys.
Speed will be automatically adjusted
to maintain a consistent note velocity.
The amount of play area to hide from the top.
Enable background animations (720p, alpha) Maintain absolute note velocity (advanced) + + + Show expert gauge (experimental) + + +
Save & Exit
} }) As I add more options things are going to be more spread apart.

Slide 192

Slide 192 text

Slide № 192 Cohesion loss Symptom: Making a simple change requires changing code that is further apart.

Slide 193

Slide 193 text

Slide № 193 Cohesion loss Symptom: Reviewing a PR requires a lot of scrolling up and down to understand.

Slide 194

Slide 194 text

Slide № 194 components/ scoreboard/ ScoreboardView.js modules/ constants.js scoreboard/ actions.js reducer.js selectors.js utils.js This can happen when files are organized by type. To add a new feature, I need to change 6 files.

Slide 195

Slide 195 text

Slide № 195 While grouping files this way makes the architecture more obvious (this may be very suitable for small projects)

Slide 196

Slide 196 text

Slide № 196 200 files 200 files 200 files 500 files But it may not scale very well to hundreds of files per type.

Slide 197

Slide 197 text

Slide № 197 Let’s try aiming for functional cohesion instead of logical cohesion. Instead of “this part selects data from the store”, “this part dispatches”, etc… No. We make it: “this part for messages”, “this part for notifications”, etc.

Slide 198

Slide 198 text

Slide № 198 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func

Slide 199

Slide 199 text

Slide № 199 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func Remove stuff not directly related to Navigation

Slide 200

Slide 200 text

Slide № 200 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func

Slide 201

Slide 201 text

Slide № 201 const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func

Slide 202

Slide 202 text

Slide № 202 class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore

Slide 203

Slide 203 text

Slide № 203 class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore

Slide 204

Slide 204 text

Slide № 204 class Navigation extends Component { render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) } {loggedIn &&

Slide 205

Slide 205 text

Slide № 205 class Navigation extends Component { render () { const loggedIn = !!this.props.currentUser return ( My app Home Explore Search {loggedIn && Notifications ({this.props.newNotificationCount}) } {loggedIn && Replace the parts that require special logic with specialized components.

Slide 206

Slide 206 text

Slide № 206 class Navigation extends Component { render () { return ( My app Home Explore Search

Slide 207

Slide 207 text

Slide № 207 class Navigation extends Component { render () { return ( My app Home Explore Search )

Slide 208

Slide 208 text

Slide № 208 class Navigation extends Component { render () { return ( My app Home Explore Search ) Example: The LoginLogoutNavItem would know how to figure out the current authentication state on its own.

Slide 209

Slide 209 text

Slide № 209 const LoginLogoutNavItem = Let’s look at it…

Slide 210

Slide 210 text

Slide № 210 const LoginLogoutNavItem = connect( )

Slide 211

Slide 211 text

Slide № 211 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), )

Slide 212

Slide 212 text

Slide № 212 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )

Slide 213

Slide 213 text

Slide № 213 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) })

Slide 214

Slide 214 text

Slide № 214 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) }) This looks pretty good! Most of the code is now concerned with auth and current user.

Slide 215

Slide 215 text

Slide № 215 User Navigation Navigation const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) }) Except for 2 lines concerned with navigation:

Slide 216

Slide 216 text

Slide № 216 User Navigation Navigation const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) }) If, in the future, we need to replace the NavItem with something else…

Slide 217

Slide 217 text

Slide № 217 User Navigation Navigation const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) }) We have to change not only Navigation, but all components used by Navigation that renders a NavItem.

Slide 218

Slide 218 text

Slide № 218 User Navigation Navigation const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) }) What can we do here?

Slide 219

Slide 219 text

Slide № 219 User Navigation Navigation const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) }) Delegate rendering logic

Slide 220

Slide 220 text

Slide № 220 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( Logout ) : ( Login ) ) }) Instead of hardcoding the rendering logic here…

Slide 221

Slide 221 text

Slide № 221 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) …let the component users supply its own rendering logic.

Slide 222

Slide 222 text

Slide № 222 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) })

Slide 223

Slide 223 text

Slide № 223 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) })

Slide 224

Slide 224 text

Slide № 224 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 225

Slide 225 text

Slide № 225 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 226

Slide 226 text

Slide № 226 const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } User Only concerned about current user and authentication Not concerned about how to render it

Slide 227

Slide 227 text

Slide № 227 User const LoginLogoutNavItem = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } Let’s change the name…

Slide 228

Slide 228 text

Slide № 228 User const Auth = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } The Auth component can now be reused in other parts of the app. (e.g. showing a piece of content only when the user is logged in)

Slide 229

Slide 229 text

Slide № 229 const Auth = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 230

Slide 230 text

Slide № 230 const Auth = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } “Render props” This technique is now officially known as:

Slide 231

Slide 231 text

Slide № 231 Back to our

Slide 232

Slide 232 text

Slide № 232 class Navigation extends Component { render () { return ( My app Home Explore Search )

Slide 233

Slide 233 text

Slide № 233 Home Explore Search ) } }

Slide 234

Slide 234 text

Slide № 234 Home Explore Search ) } }

Slide 235

Slide 235 text

Slide № 235 Home Explore Search ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 236

Slide 236 text

Slide № 236 Home Explore Search ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 237

Slide 237 text

Slide № 237 Home Explore Search Login } renderLoggedIn={(user, logout) => Logout } /> ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 238

Slide 238 text

Slide № 238 Home Explore Search Login } renderLoggedIn={(user, logout) => Logout } /> ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 239

Slide 239 text

Slide № 239

Slide 240

Slide 240 text

Slide № 240 const Auth = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }

Slide 241

Slide 241 text

Slide № 241 const Auth = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } Objective: Connect UI to data source.

Slide 242

Slide 242 text

Slide № 242 const Auth = connect( state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } “Data connector component” At Taskworld, we call this kind of components:

Slide 243

Slide 243 text

Slide № 243 Navigation Recap

Slide 244

Slide 244 text

Slide № 244 Navigation As components grow bigger it can be hard to maintain.

Slide 245

Slide 245 text

Slide № 245 NavigationContainer NavigationView The most obvious way to split them. But this could sometimes lead to cohesion loss, especially when one part would not make sense without the other.

Slide 246

Slide 246 text

Slide № 246 Navigation Auth Unread MessagesData Unread NotificationsData We can try to split by functional goals:

Slide 247

Slide 247 text

Slide № 247 Navigation Auth Unread MessagesData Unread NotificationsData If done right, it’s also easier to re-use these components in other parts of the app.

Slide 248

Slide 248 text

Slide № 248 Conclusions

Slide 249

Slide 249 text

Slide № 249 When code reviewing Consistency Readability Logical correctness We usually look for: These are relatively simpler to review.

Slide 250

Slide 250 text

Slide № 250 When code reviewing Consistency Readability Logical correctness Smells Subtle. It’s easy to see the code as “correct” but we may miss the subtle design issues in our code, unless we’re aware of them. To build maintainable software, I encourage everyone to develop the sense of smells and be mindful of them!

Slide 251

Slide 251 text

Slide № 251 I don’t know of any way to detect these smells automatically. Today I showed you 2 kinds of smells. These are the kinds of smells I encountered the most often.

Slide 252

Slide 252 text

Slide № 252 When writing/reviewing code, try looking for these smells.

Slide 253

Slide 253 text

Slide № 253 Not all smells are bad. Last but not least…

Slide 254

Slide 254 text

Slide № 254 Not all smells are bad. Example: Duplicated code a well known code smell

Slide 255

Slide 255 text

Slide № 255 Not all smells are bad. Example: Duplicated code can sometimes be easier to read because they are more explicit and require less amount of abstraction

Slide 256

Slide 256 text

Slide № 256 That’s why I always duplicate my slides instead of using animations!

Slide 257

Slide 257 text

Slide № 257 Everything is a trade-off. In the end, Examples: Smaller modules → More modules. Reducing amount of code you maintain → More 3rd party dependencies. We have to make a lot of choices. We have to strike a good balance.

Slide 258

Slide 258 text

Slide № 258 Look for feedback. To see if we made the right trade-offs, After all, we make code maintainable so that other people can maintain it. So listen to them — look for their feedback!

Slide 259

Slide 259 text

Slide № 259 Thank you!