Skip to content
This repository was archived by the owner on Feb 14, 2022. It is now read-only.

Add expectation tier function for non-members #23

Merged
merged 3 commits into from
Nov 27, 2019

Conversation

DevSDK
Copy link
Contributor

@DevSDK DevSDK commented Nov 6, 2019

Before this function implemented, user cannot know where they are
and what are the proper problems without tier

This function works following steps

  1. Parse the solved problem in user page
  2. Get Level information from Api server
  3. Calculate the total exp point from level information
  4. Display expectation tier

Thanks
Seokho

Before this function implemented, user cannot know where they are
and what are the proper problems without tier

This function works following steps

1. Parse the solved problem in user page
2. Get Level information from Api server
3. Calculate the total exp point from level information
4. Display expectation tier

Signed-off-by: Seokho Song <0xdevssh@gmail.com>
@shiftpsh shiftpsh added enhancement New feature or request good first issue Good for newcomers labels Nov 7, 2019
Copy link
Member

@shiftpsh shiftpsh left a comment

Choose a reason for hiding this comment

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

Thank you for your pull request, I've reviewed your code and requested some changes.

Although, this is extremely inefficient for users with like >1000 solved problems; it will wait for the plugin to process >1000 requests. Maybe in future I'll make a separate endpoint that calculates this information.

+ "</a>"

if(!userData && isShowUserTempTier)
{
Copy link
Member

Choose a reason for hiding this comment

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

Please use K&R style brackets.

if(!userData && isShowUserTempTier)
{
var acceptPanel = document.querySelector(".panel.panel-default")
var acceptProblems = acceptPanel.querySelectorAll(".problem_number>a")
Copy link
Member

Choose a reason for hiding this comment

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

Since BOJ and solved.ac calculates accepted problems differently, the actual experience points calculated from this method will differ from official calculation. See https://solved.ac/faq/

Copy link
Contributor Author

@DevSDK DevSDK Nov 7, 2019

Choose a reason for hiding this comment

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

Could you explain more details?
I'm not sure what I need to change on this part.

Copy link
Member

Choose a reason for hiding this comment

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

If problem is marked with tag 'Preparing Judge', it should not be calculated as accepted.

Copy link
Contributor Author

@DevSDK DevSDK Nov 7, 2019

Choose a reason for hiding this comment

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

Does api server respond 'level 0' if the problem that was tagged 'preparing judge'?

const levelData = [320, 480, 912, 1642, 2791, 4465, 6698, 9913, 14472, 20840, 29593,
41431, 57588, 79472, 108877, 148072, 200638, 270862, 364309, 488174,
651712, 866777, 1148479, 1515993, 1993531, 2611525, 3408040, 4430452,
5737436, 7401292, 9510661]
Copy link
Member

Choose a reason for hiding this comment

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

Level experience points are subject to change, it should NOT be declared as constants. Please use https://api.solved.ac/exp_table.php

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we had an api document(or just list), it would be great to develop some of functions.

Copy link
Member

Choose a reason for hiding this comment

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

I will document APIs in the future, but it's a shame that I'm currently too busy to make and publish documented APIs.

const expTable = [0, 4800, 15740, 38720, 83400, 163700, 298000, 785400, 1202800, 1795000, 2834700,
4276000, 6261000, 8982000, 12704000, 18796000, 26842000, 37941000, 52792000,
720000000, 152000000, 213000000, 294000000, 380000000, 639000000, 1000000000,
1200000000, 1500000000]
Copy link
Member

Choose a reason for hiding this comment

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

Tier bounds are also subject to change, hence it should NOT be declared as constants. Currently we don't have API endpoints for this, I'll fix it later.

Copy link
Member

Choose a reason for hiding this comment

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

Yes it does, but its experience points has to be calculated as zero.

Copy link
Member

Choose a reason for hiding this comment

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

Please change this to use https://api.solved.ac/exp_cap.php.


<div class="option_item" data-key="show_userpage_temp_tier">
<span class="option_caption">
유저페이지 임시티어 보기
Copy link
Member

Choose a reason for hiding this comment

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

Please change it to "사용자 페이지에서 비공식 티어 보기".


var expectLevel = getExpectLevelFromExpPoint(totalExpPoint)
newRowHeader.innerText = "solved.ac 임시티어"
newRowDescription.innerHTML = "<a href=\"https://boj.com/" + userId + "\">"
Copy link
Member

Choose a reason for hiding this comment

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

What's with boj.com? It links to some random premium domain broker site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's my mistake, I'll fix to boj.kr or acmicpc.net.

@DevSDK
Copy link
Contributor Author

DevSDK commented Nov 7, 2019

Please check my commit that your request has been resolved!

1. Fix missed coding style code to k&r style
2. Replace option menu
3. Replace constant exp point table to server based table
4. Fix URL on expectation tier
5. Ignore level 0 problem to calculate total exp point

Signed-off-by: Seokho Song <0xdevssh@gmail.com>
@DevSDK
Copy link
Contributor Author

DevSDK commented Nov 8, 2019

Fix missed K & R style from e211899 to c467e18; a blank space.
:)

@DevSDK DevSDK requested a review from shiftpsh November 8, 2019 03:22
@shiftpsh
Copy link
Member

I'm currently too busy preparing problems for Sogang Programming Contest. I'll take a look when I'm not busy. Thanks.

@DevSDK
Copy link
Contributor Author

DevSDK commented Nov 12, 2019

Ah Okay! Take a time!

Copy link
Member

@shiftpsh shiftpsh left a comment

Choose a reason for hiding this comment

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

Please fix some trivial formatting errors, and I think we're good to go.

const expTable = [0, 4800, 15740, 38720, 83400, 163700, 298000, 785400, 1202800, 1795000, 2834700,
4276000, 6261000, 8982000, 12704000, 18796000, 26842000, 37941000, 52792000,
720000000, 152000000, 213000000, 294000000, 380000000, 639000000, 1000000000,
1200000000, 1500000000]
Copy link
Member

Choose a reason for hiding this comment

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

Please change this to use https://api.solved.ac/exp_cap.php.

4276000, 6261000, 8982000, 12704000, 18796000, 26842000, 37941000, 52792000,
720000000, 152000000, 213000000, 294000000, 380000000, 639000000, 1000000000,
1200000000, 1500000000]
for(i=0;i<expTable.length-1;i++) {
Copy link
Member

Choose a reason for hiding this comment

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

Code is not well-formatted; there must be spaces between operands and operators.

Add spaces between operands and opraters
Replace from constant exp table to exp table fetched from api

Signed-off-by: Seokho Song <0xdevssh@gmail.com>
@DevSDK
Copy link
Contributor Author

DevSDK commented Nov 27, 2019

And here we go!

@DevSDK DevSDK requested a review from shiftpsh November 27, 2019 13:34
Copy link
Member

@shiftpsh shiftpsh left a comment

Choose a reason for hiding this comment

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

LGTM

@shiftpsh shiftpsh merged commit b7d4093 into solved-ac:master Nov 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants