Barney commentary#4
Conversation
|
BTW when looking at a commit or a pull request's diff, append |
| class Answer extends React.Component{ | ||
| constructor(props) { | ||
| super(props); | ||
| const Answer = props => ( |
There was a problem hiding this comment.
Where possible, functional components are IMO better because you get an immediate assurance that there is no lifecycle and no state, which makes it easier to reason about contents.
Un-bracketed arrow functions are even better because they are 'point-free', ie there are no internal references, just a return expression. Previously there were 2 points: declare a variable to define the options, then return a block of content which references the options. I prefer this because it's less moving parts to remember – some people dislike it on the grounds it encourages that any logic (ie the options.map) gets put in the content (I don't see a problem with that 🤷♂️ ).
| name='rad' | ||
| id={`option_${i}`} | ||
| onChange={() => { | ||
| props.handleRadio(i) |
There was a problem hiding this comment.
I prefer using anonymous event handlers in the view and calling function references within that – this avoids the necessity of re-binding class methods elsewhere because we explicitly avoid the this re-binding that comes from event handler assignment. Previously, the parent component's this binding for handleRadio was over-written by the binding here.
| type="submit" | ||
| className='waves-effect waves-light btn-large submit-button visible' | ||
| onClick={props.onClickCallback} | ||
| disabled={!props.selectionMade} |
There was a problem hiding this comment.
Boolean attribute expressions can be expressed like this – I love it.
1 participant
Just putting this up here as a kind of dynamic code review