Skip to content

ready for checking commit - #1

Open
Dzemlianoi wants to merge 5 commits into
masterfrom
ready_branch
Open

ready for checking commit#1
Dzemlianoi wants to merge 5 commits into
masterfrom
ready_branch

Conversation

@Dzemlianoi

Copy link
Copy Markdown
Owner

No description provided.

Comment thread lib/library.rb Outdated
top_books_readers.push(order.reader)
end
end
top_books_readers.uniq.size

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplify it please

Comment thread lib/library.rb Outdated

def sort_grouped(data)
sorted = data.sort_by { |_, v| v.size }
sorted.reverse.to_h

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary to use local variable here

Comment thread lib/order.rb Outdated
@date = date
else
raise 'Unnormed data'
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do it without else?

Comment thread lib/library.rb
def top_books_statistics(number)
raise 'Annormal quantity of books' if number > orders.count
top_n_books = sort_by(:book, number)
top_n_books.map{|(_,orders)| orders}.flatten.map(&:reader).uniq.count

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use without local here

Comment thread lib/loader.rb
@authors = data[:authors]
@books = data[:books]
@readers = data[:readers]
@orders = data[:orders]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we unify this

Repository owner deleted a comment from yamnich Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants