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.

Sign up for GitHub

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

Jump to bottom

[SonarCloud] Legacy auction endpoint #1017

Merged
merged 9 commits into from
Dec 3, 2019
Merged

[SonarCloud] Legacy auction endpoint #1017

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6003ab7
reducing conditionals in endpoints/auction.go:auction
Aug 27, 2019
0853246
further refactoring
Aug 28, 2019
618d571
Correcting legacy endpoint sonarcloud issues
Aug 29, 2019
ff415d1
Got rid of comments
Aug 29, 2019
2d907a1
Scott's comments first batch
Oct 16, 2019
31d8a24
Scott's comments batch 2
Oct 16, 2019
712a528
Test case corrected
Oct 17, 2019
70b0d5b
refactored so function gets executed with more precision
Nov 4, 2019
df2f797
correcting for loop as per Mansi's comments
Nov 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions endpoints/auction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,14 +246,7 @@ bidLoop:
for _, bid := range bids {
if isUndimensionedBanner(bid) {
for _, adunit := range bidder.AdUnits {
if isBidIDAndCodeEqual(&adunit, bid) {
if len(adunit.Sizes) == 1 {
bid.Width, bid.Height = adunit.Sizes[0].W, adunit.Sizes[0].H
finalValidBids[finalBidCounter] = bid
finalBidCounter = finalBidCounter + 1
} else if len(adunit.Sizes) > 1 {
glog.Warningf("Bid was rejected for bidder %s because no size was defined", bid.BidderCode)
}
if copyBannerDimensions(&adunit, bid, finalValidBids, &finalBidCounter) {
continue bidLoop
}
}
Expand All @@ -269,8 +262,21 @@ func isUndimensionedBanner(bid *pbs.PBSBid) bool {
return bid.CreativeMediaType == "banner" && (bid.Height == 0 || bid.Width == 0)
}

func isBidIDAndCodeEqual(adunit *pbs.PBSAdUnit, bid *pbs.PBSBid) bool {
return adunit.BidID == bid.BidID && adunit.Code == bid.AdUnitCode
func copyBannerDimensions(adunit *pbs.PBSAdUnit, bid *pbs.PBSBid, finalValidBids []*pbs.PBSBid, finalBidCounter *int) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

May I ask what's the reason behind using pointers here for finalValidBids and finalBidCounter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

finalValidBids was declared in line 243 as a dynamic array of pointers, therefore, I'm passing it as a parameter to copyBannerDimensions as such:

243     finalValidBids := make([]*pbs.PBSBid, len(bids))
   .
   .
265 func copyBannerDimensions(adunit *pbs.PBSAdUnit, bid *pbs.PBSBid, finalValidBids []*pbs.PBSBid, finalBidCounter *int) bool {

On the other hand finalBidCounter serves as a counter of the for loop in line 246 and gets modified inside copyBannerDimensions (line 272), therefore it needs to be passed as a reference

var bidIDEqualsCode bool = false

if adunit.BidID == bid.BidID && adunit.Code == bid.AdUnitCode && adunit.Sizes != nil {
if len(adunit.Sizes) == 1 {
bid.Width, bid.Height = adunit.Sizes[0].W, adunit.Sizes[0].H
finalValidBids[*finalBidCounter] = bid
*finalBidCounter += 1
} else if len(adunit.Sizes) > 1 {
glog.Warningf("Bid was rejected for bidder %s because no size was defined", bid.BidderCode)
}
bidIDEqualsCode = true
}

return bidIDEqualsCode
}

// sortBidsAddKeywordsMobile sorts the bids and adds ad server targeting keywords to each bid.
Expand Down