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.
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
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.
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.
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.
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.
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
programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … As I said, there are many contributing factors:
programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … Tech stuffs The things that I listed here are…
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
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
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.
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.
/> <UserAvatar size='large' … /> 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?
/> <UserAvatar size='large' … /> A UserAvatar component <UserAvatar size='message' … /> We did the most obvious thing: Does this smell a bit to you? Let’s keep going on!
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.
… } 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.
… } 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…
= { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Now, Navigation is just a presentational component. We need to connect to the Redux store.
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.
{ return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <LoginLogoutNavItem /> </Nav> </Navbar> ) Example: The LoginLogoutNavItem would know how to figure out the current authentication state on its own.
state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) We have to change not only Navigation, but all components used by Navigation that renders a NavItem.
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
({ 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)
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!
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.
dtinth
oniak3ibm
michaelzikes
ivargrimstad
yoheimuta
soogie
fufuhu
kazk1018
mixi_engineers
josepaumard
nonews
djain2405
paulk
michaelherold
jnunemaker
chrislema
rocio
deanohume
garrettdimon
reverentgeek
sferik
jeffersonlam
lara
malarkey
destraynor
