oleksiimikhno / RubyHW

0 stars 0 forks source link

Feedback HW4 #6

Closed MaksHostroushko closed 1 year ago

MaksHostroushko commented 1 year ago

Looks really great! Thanks for your work But let's add some improvements

  1. Please, add correct spaces for tags in this file

  2. Let's write all modules in a single row. I mean, include GameEngine, ActionUser https://github.com/xiosky90/RubyHW/blob/7c6adaf2df0236603e5c2958acea32d296a9c136/HW4/Tamagotchi/app/tamagotchi.rb#L16..L20

  3. Try to don't write multiple conditions in one row for if/else. In this case it's a hard to understand all conditions Actual Result: game_message("#{@name} eat you..", 'alert') unless @name == 'Monster' && @belly_value < 5 Expected Result:

    unless @name == 'Monster' && @belly_value < 5`
    game_message("#{@name} eat you..", 'alert') 
    end
  4. You can save result of comparing to variable instead of extra logic @sleep_need = @time == 10 instead of https://github.com/xiosky90/RubyHW/blob/7c6adaf2df0236603e5c2958acea32d296a9c136/HW4/Tamagotchi/app/tamagotchi.rb#L226

  5. Please, change the naming for these methods. For now, it looks really unreadable https://github.com/xiosky90/RubyHW/blob/7c6adaf2df0236603e5c2958acea32d296a9c136/HW4/Tamagotchi/app/tamagotchi.rb#L236..L242 For example, use sum_of_values

  6. You can use if/else instead of case if you don't have a lot of duplicated conditions https://github.com/xiosky90/RubyHW/blob/7c6adaf2df0236603e5c2958acea32d296a9c136/HW4/Tamagotchi/app/controllers/action_user.rb#L16..L22

oleksiimikhno commented 1 year ago

Hello, thanks for your review.

  1. Fixed
  2. Fixed, but rubocop warning like Putincludemixins in separate statements. (convention:Style/MixinGrouping) https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/MixinGrouping if change include to extend, warning not disappear, it's normal?
  3. Before i am use if/else but rubocop say it's problem Class: RuboCop::Cop::Style::IfUnlessModifier return code to if/else
  4. Thanks fixed.
  5. Remove inc_value and dec_value methods.
  6. Rework to ternary operator. Thanks again for your work.