Development #1
Development #1
Conversation
lib/codebreaker/entities/game.rb
Outdated
| MAX_CODE_NUM = 6 | ||
| DIGITS_NUM = 4 | ||
|
|
||
| attr_reader :user, :difficulty, :secret_code, :date, :hints_list, :attempts, :hints |
There was a problem hiding this comment.
I would remove the secret_code from here
There was a problem hiding this comment.
it must be here because console and web app must return secret_code if user win or lose
There was a problem hiding this comment.
And if is a user not win and not lose?
There was a problem hiding this comment.
what do you mean?
secret_code is necessary to show only if user win or lose
so, it must be in attr_reader
| GUESS_IS_NOT_INTEGER = 'Guess should be Integer class'.freeze | ||
| DIGITS_COUNT_ERROR = 'Invalid digits count'.freeze | ||
| DIGIT_RANGE_ERROR = 'Digit is not in a range'.freeze |
There was a problem hiding this comment.
If this is displayed then add I18n
| def self.validate(guess) | ||
| raise ValidationError, GUESS_IS_NOT_INTEGER unless guess[/^\d+$/] | ||
| raise ValidationError, DIGITS_COUNT_ERROR unless guess.size == Game::DIGITS_NUM | ||
| raise ValidationError, DIGIT_RANGE_ERROR unless guess.chars.all? do |num| | ||
| num.to_i.between? Game::MIN_CODE_NUM, Game::MAX_CODE_NUM | ||
| end | ||
| end |
There was a problem hiding this comment.
Can carry over to 'validation' module
| answer << RIGHT_ANSWER_SYMBOL | ||
| @secret_code[index], @user_input[index] = nil |
There was a problem hiding this comment.
Can write a separate function
lib/codebreaker/modules/validator.rb
Outdated
| DIFFICULTY_ERROR = 'No such difficulty'.freeze | ||
| NAME_IS_NOT_STRING_ERROR = 'Name should be an instance of String'.freeze | ||
| SHORT_NAME_ERROR = 'Name is too short'.freeze | ||
| LONG_NAME_ERROR = 'Name is too long'.freeze |
| validate_name_min_length(@name, @errors) if @errors.empty? | ||
| validate_name_max_length(@name, @errors) if @errors.empty? |
There was a problem hiding this comment.
it is need here
for example if User.new(322) there will be no sense to check @name.length because there is no method for 322:Integer
There was a problem hiding this comment.
Ok. Then what about one validation function for a user?
There was a problem hiding this comment.
no
my validations in module
functions in module should be suitable for anything, not only for user
Something like that
| @user_input.compact! | ||
| @secret_code.compact! | ||
| near_matchers = @secret_code & @user_input | ||
| near_matchers.size.times { answer << WRONG_ANSWER_SYMBOL } |
There was a problem hiding this comment.
I suggest replacing this code with:
Array.new(near_matchers.size) { '-' }.joinand then you don't need answer = '' and answer at all in this case
| File.new(file_path, 'w') unless File.exist?(file_path) | ||
| end | ||
|
|
||
| def game_to_h(game) |
There was a problem hiding this comment.
Is rubocop ok with this function length?
spec/codebreaker/difficulty_spec.rb
Outdated
| let(:level) { 'easy' } | ||
| let(:invalid_difficulty) { described_class.new(invalid_level) } | ||
| let(:invalid_level) { 'qwerty' } | ||
| let(:difficulty_constant) do |
There was a problem hiding this comment.
You may just get this constant from the code with Difficulty::DIFFICULTIES, it will be better because if something changes in constant you don't have to change tests
| require 'spec_helper' | ||
|
|
||
| RSpec.describe Codebreaker::GuessChecker do | ||
| let(:plus_symbol) { '+' } |
There was a problem hiding this comment.
Same here it's better to use constants
| @@ -0,0 +1,25 @@ | |||
| RSpec.describe Codebreaker::StatisticsService do | |||
| let(:game) { Codebreaker::Game.new(Codebreaker::User.new('Mechetel'), Codebreaker::Difficulty.new('hell')) } | |||
| let(:path) { './lib/codebreaker/test.yaml' } | |||
There was a problem hiding this comment.
And here, constant will be better
There was a problem hiding this comment.
but here :path is example variable and there is no CONSTANT for it
.rubocop.yml
Outdated
| Metrics/BlockLength: | ||
| Exclude: | ||
| - 'spec/codebreaker/**/*' | ||
|
|
||
| Metrics/ModuleLength: | ||
| Exclude: | ||
| - 'spec/codebreaker/**/*' | ||
|
|
||
| Lint/AmbiguousBlockAssociation: | ||
| Exclude: | ||
| - 'spec/codebreaker/**/*' | ||
|
|
||
| RSpec/AnyInstance: | ||
| Exclude: | ||
| - 'spec/codebreaker/**/*' |
There was a problem hiding this comment.
done, but BlockLength is left
codebreaker.gemspec
Outdated
| spec.add_development_dependency 'pry-byebug' | ||
| spec.add_development_dependency 'i18n' | ||
| spec.add_development_dependency 'rake' | ||
| spec.add_development_dependency 'fasterer' | ||
| spec.add_development_dependency 'rspec' | ||
| spec.add_development_dependency 'rubocop' | ||
| spec.add_development_dependency 'rubocop-performance' | ||
| spec.add_development_dependency 'rubocop-rspec' | ||
| spec.add_development_dependency 'rubycritic' | ||
| spec.add_development_dependency 'simplecov' | ||
|
|
spec/codebreaker/difficulty_spec.rb
Outdated
| end | ||
|
|
||
| it 'adds DifficultyError to errors' do | ||
| invalid_difficulty.valid? |
spec/codebreaker/difficulty_spec.rb
Outdated
| end | ||
|
|
||
| it 'adds nothing to errors' do | ||
| difficulty.valid? |
spec/codebreaker/user_spec.rb
Outdated
| end | ||
|
|
||
| it 'adds longnameerror to errors' do | ||
| long_name_user.valid? |
No description provided.