-
-
Notifications
You must be signed in to change notification settings - Fork 12
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
Setting Up a Repo Function Which Leverages Raw() to Push Process of Grouping Bets Table to Create a Market:User Map #352
base: main
Are you sure you want to change the base?
Conversation
… by building a market:user map and counting all entries within it.
usersByMarket := make(map[string][]string) | ||
|
||
// create map of market_id's and users for all bets |
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.
I'd switch the comment and the map making lines, and condense them to a single block.
// comment here
usersByMarket:= ....
for _, bet ...
This logic is all together so grouping them under the comment makes it clear that these go together.
// Count total first-time bets (users across markets) | ||
var totalFirstTimeBets int64 | ||
for _, users := range usersByMarket { | ||
totalFirstTimeBets += int64(len(users)) | ||
} |
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.
Is this counting unique users per market for all markets? i.e. double counts of users are permitted as long as users are double counted in different markets?
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.
Correct!
The reason being, as described above ... https://github.com/openpredictionmarkets/socialpredict/pull/352/files#r1817878774
We're trying to calculate fees.
@@ -0,0 +1,48 @@ | |||
package repository |
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.
I still find this term repository
to be confusing. I've only ever heard it used to refer to a code base not a wrapper for a database. Do you have any examples of it being used this way?
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.
Stated this on discord, putting it here as well for transparency.
https://github.com/openpredictionmarkets/socialpredict/pull/352/files ... Here is some more background information on the concept of a repository pattern, specifically in golang. https://github.com/jorzel/go-repository-pattern ... related to this, is the concept of Domain Driven Design (DDD). We are already following a DDD in that our business logic is in Golang and our Application logic is in React. So what the repository pattern is, is an Infrastructure Layer which basically says, in an authoritarian way, "this is how you interact with the database," ... with the idea that, we're designing efficiency from the start, reducing refactoring down the line that we may need to deal with if we consider that our bet table should grow geometrically with the number of users. E.g. the way we are typically doing queries now, is we are pulling the entire bet table, and then having Golang run through that table and having faith that Golang is fast enough to deal with this. So while Golang is extremely powerful, my concern would be if we have a large number of users simultaneously, an event, let's say in the case of Kenyon, election day, and all of those users are pulling the entire table again and again with different functions they are doing, that becomes an I/O problem and a memory problem. Though admittedly as we discussed, I don't know how much of a memory problem that will become. Also admittedly, we could just tell users, "you need to buy more hardware for the time being."
// Execute raw SQL query to group by both market_id and username | ||
// gorm GROUP does not allow two inputs, so this has to be a raw function | ||
result := repo.db.Raw(` | ||
SELECT market_id, username |
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.
If we did SELECT DISTINCT ....
would that return the desired result?
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.
I don't believe it does, I remember trying that, would have to go back and check this. I'm trying to get a COUNT value which we can then, "Scan" (using Gorm terminology) into an int64.
return &BetsRepository{db: db} | ||
} | ||
|
||
func (repo *BetsRepository) FirstTimeBets() (int64, error) { |
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.
I'm not really clear on what this is supposed to return/do? Some of my follow on comments are trying to figure that out.
Can you write a comment explaining what this function is supposed to do?
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.
Good question. The FirstTimeBets()
function is meant to quickly and efficiently count all first time bets on a particular market.
This number, an int64
is then multiplied by the https://github.com/openpredictionmarkets/socialpredict/blob/main/backend/setup/setup.yaml#L16 ... initialBetFee
amount to calculate the total number of first time bet fees obtained from the market.
This can be used either in payment to the market maker upon resolution of the market, or within the sitewide stats calculation api which helps ensure that no untracked money is created in the system.
Re, This Discussion:
https://github.com/orgs/openpredictionmarkets/projects/9/views/1?pane=issue&itemId=80921952