LilasCorner / jlootbox

Other
1 stars 0 forks source link

Consider reworking platform.findFavorite #16

Open JohnTMurphy-NIU opened 2 years ago

JohnTMurphy-NIU commented 2 years ago

This should not be a static method; it should be initialized so that the list of players is initialized once. The two loops (currently lines 118 and 135) should be one loop, with the comparison between both degree and avgHistValue, rather than creating a second list, populating it, and looping through it.

JohnTMurphy-NIU commented 2 years ago

I would look at this method more closely, because I remember we have discussed something a little bit subtle. Right now it looks like there might be an error in line 120; you are comparing against 'favorite.avgHistValue' instead of against '((Player)fav).avgHistValue'. So what will happen is this: Assume your previous 'favorite' had an in-degree of 2 and an avgHistValue of 1. If you find a player "A" with a new high value for in-degree (say, 3), that player becomes 'fav'; assume the avgHistValue is 3. Then you find another player "B" with the same in-degree value, which you consider replacing as 'fav'. "B" has an avgHistValue of 2. According to your comparison here, "B" becomes 'fav', because its avgHistValue is higher than the previous favorite (2 > 1), even though it is lower than the "A". I remember, though, that we also discussed something about keeping the previous favorite if it remained in the top set of in-degree, even if its avgHist went down. So I'm unsure if this method is correct or not.