{"comment":{"url":"https://api.github.com/repos/jeroenbreen/newArch/comments/11428683","html_url":"https://github.com/jeroenbreen/newArch/commit/7f7f495dc7f4fea3deaa9e46518cf46a26ec2187#commitcomment-11428683","id":11428683,"user":{"login":"graphicore","id":393132,"avatar_url":"https://avatars.githubusercontent.com/u/393132?v=3","gravatar_id":"","url":"https://api.github.com/users/graphicore","html_url":"https://github.com/graphicore","followers_url":"https://api.github.com/users/graphicore/followers","following_url":"https://api.github.com/users/graphicore/following{/other_user}","gists_url":"https://api.github.com/users/graphicore/gists{/gist_id}","starred_url":"https://api.github.com/users/graphicore/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graphicore/subscriptions","organizations_url":"https://api.github.com/users/graphicore/orgs","repos_url":"https://api.github.com/users/graphicore/repos","events_url":"https://api.github.com/users/graphicore/events{/privacy}","received_events_url":"https://api.github.com/users/graphicore/received_events","type":"User","site_admin":false},"position":54,"line":54,"path":"app/lib/ui/specimen/specimen-tools/size-rope/size-rope-directive.js","commit_id":"7f7f495dc7f4fea3deaa9e46518cf46a26ec2187","created_at":"2015-05-29T14:45:54Z","updated_at":"2015-05-29T14:45:54Z","body":"Splitting this stuff up into smaller functions was a good start already.\r\nWhat you should also do is to keep the number of variables in the outer scope (in this case the closure created by the link function) lower. Such as `lineTemp` in this case or `svgTemp`.\r\n\r\nIn general it is advisable to keep the number of side-effects of a function at a minimum, and if there are any, make it explicit. A \"side effect\" means you run a function and the function changes some state that is not in the function, like setting `screenX` `screenY` in `createTemplayer`.\r\n\r\nFor an object method, side effects are often wanted, by changing the values of the properties of `this`. In a way this is the same thing like what you do here, by using a closure scope that contains some shared state variables. However, it is harder to read, especially when there are some values that should not be used as state variables (because they are never used outside of the function that writes them).\r\n\r\nYou can use the arguments and the return values of functions to greatly reduce the side effects they have. Also to make it nicer to read, it wold be good if you enclose the state variables into a comment that says `/* these are shared state variables*/`, or you use one `state` object that contains all the state variables its properties, then all of this is nicely at one place.\r\n\r\nLooking long enough at the code I now know how it works."}}