Game feat/progress

[liam-m]

@Beornwulf review:

• I said about using the null object pattern. Up to you if you want to do that - would be something to talk about in the report, and shows we're using some of the patterns covered in the course. On the other hand, I think it's probably a bit heavy-handed for avoiding 1 conditional, and we can talk about why we haven't used the null object pattern in the report instead?
• Tests - just looking at coverage (which is easy but probably not the best way to decide what needs doing), these are the changes: 5 lines missed in routes/leaderboard.rb, 3 in routes/user.rb, 7 in models/user.rb. Could you add tests for these? It's probably better to go from the specification and check each case there has a test, but I'm not familiar with the spec
• There seems to be duplication of the required_value numbers (1, 5, 10) between badge.rb and user.rb. Can this be removed?
• I think the before blocks (1, 2) are redundant as they're already in auth.rb
• Could the commented out code here be removed?
• Is this file as expected or does it need finishing?
• Perhaps 'all badges earned' would be better than the negative here

[Beornwulf]

to answer in order:

• definitely overkill, will attempt to include in report
• we don't get marked on the code itself iirc, spending precious time adding tests probably not vital?
• these were there for eventual genericisation of the code
• I disagree, pretty sure we need that for any pages that are behind the login wall
• done
• never ended up being included due to time restraints
• done

[liam-m]

• Okay great, so just something along the lines of "we considered using... advantages... disadvantages.. on balance disadvantages outweigh advantages"
• I think tests are needed for Execution, formative feedback says he looked at the repo. I think adding tests gives us something to talk about in Execution, showing it works as designed
• So are we going to genericise it?
• We do need it, I don't think we need it 3 times. Try removing it, I think everything will keep working, we're just overwriting @user several times, doing the same thing each time
• Cheers
• Remove it?
• Cheers

[liam-m]

In terms of the increment_quizzes function, I think it could be refactored to get all quiz badges, loop through them and compare self['quizzes_completed'] against badge.required_value - reduces the duplication and probably complexity, duplication, etc.