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

Fix the bug of multiple symbol(#267) #269

Closed
wants to merge 2 commits into from
Closed

Fix the bug of multiple symbol(#267) #269

wants to merge 2 commits into from

Conversation

XanderShum
Copy link
Contributor

Comment on lines 23 to 30
symbols := "["
for i, pair := range pairs {
if i != 0 {
symbols += ","
}
symbols += "\"" + pair + "\""
}
symbols += "]"
Copy link

Choose a reason for hiding this comment

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

Just formatted your code in a cleaner way. You also need to import strings package.

Suggested change
symbols := "["
for i, pair := range pairs {
if i != 0 {
symbols += ","
}
symbols += "\"" + pair + "\""
}
symbols += "]"
if len(pairs) == 0 {
s.symbols = "[]"
} else {
s.symbols = "[\"" + strings.Join(pairs, "\",\"") + "\"]"
}
return s

@adshao adshao closed this Jun 30, 2021
@adshao adshao reopened this Jun 30, 2021
@icamys
Copy link
Contributor

icamys commented Oct 5, 2021

@adshao any updates on this fix?

Copy link
Contributor

@icamys icamys left a comment

Choose a reason for hiding this comment

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

The tests also should be updated. In the exchange_info_service_test.go the following line:

"symbols": symbols,

should be:

"symbols": "[\"ETHBTC\",\"LTCBTC\"]",

adshao pushed a commit that referenced this pull request Oct 13, 2021
…#315)

* fix multiple symbols concatenation

Fixes #267

* Update exchange_info_service_test.go
@icamys
Copy link
Contributor

icamys commented Oct 13, 2021

@adshao The related bug (#267) is already fixed. I believe this PR can be closed.

@adshao
Copy link
Owner

adshao commented Oct 13, 2021

thanks @icamys

@adshao adshao closed this Oct 13, 2021
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.

None yet

4 participants