Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

roman-haidarov
Copy link

No description provided.

Copy link
Collaborator

@spajic spajic left a 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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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."
Copy link
Collaborator

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(', ')
Copy link
Collaborator

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]
Copy link
Collaborator

Choose a reason for hiding this comment

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

я понял концепцию обойтись без дополнительных переменных, но в данном случае это приводит к избыточной работе на каждом шаге

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