-
Notifications
You must be signed in to change notification settings - Fork 181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimization parser to json #144
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
без case-study ход мысли не понятен, это на самом деле самое главное в этом задании - потренироваться работать шаг за шагом, отмечая что как сыграло
all_sessions = sessions.values.flatten | ||
|
||
all_sessions.each do |session| | ||
browsers_map[session[:browser].upcase] ||= 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
два раза делаем session[:browser].upcase
|
||
all_sessions.each do |session| | ||
browsers_map[session[:browser].upcase] ||= 0 | ||
browsers_map[session[:browser].upcase] += 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
для уникальных браузеров можно Set использовать
attributes = user | ||
user_object = User.new(attributes: attributes, sessions: sessions[user[:id]]) | ||
users_objects << user_object | ||
progressbar.increment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
это лучше опциональным, тк может тормозить, особенно если увеличивать на каждой итерации (а не раз в 10/100/1000...)
# Даты сессий через запятую в обратном порядке в формате iso8601 | ||
report[:usersStats][user_key][:dates] ||= [] | ||
dates = report[:usersStats][user_key][:dates] | ||
report[:usersStats][user_key][:dates] = (dates << cols[5]).sort.reverse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sort создаёт новый массив, reverse - ещё один
# Собираем количество времени по пользователям | ||
report[:usersStats][user_key][:totalTime] ||= '0 min.' | ||
total_time = report[:usersStats][user_key][:totalTime] | ||
report[:usersStats][user_key][:totalTime] = "#{total_time.split(' ').first.to_i + cols[4].to_i} min." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
сначала собирать строку, потом разбирать выглядит не очень оптимально
report[:usersStats][user_key][:browsers] ||= '' | ||
browsers_arr = report[:usersStats][user_key][:browsers].split(', ') | ||
current_browser = cols[3].upcase | ||
report[:usersStats][user_key][:browsers] = (browsers_arr << current_browser).sort.join(', ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
каждый раз пересортировывать и пересобирать строку - не оч
|
||
# Даты сессий через запятую в обратном порядке в формате iso8601 | ||
report[:usersStats][user_key][:dates] ||= [] | ||
dates = report[:usersStats][user_key][:dates] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
я понял концепцию обойтись без дополнительных переменных, но в данном случае это приводит к избыточной работе на каждом шаге
No description provided.