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

Query method implementations #3

Open
wants to merge 7 commits into
base: feat/bip39
Choose a base branch
from
Open

Query method implementations #3

wants to merge 7 commits into from

Conversation

psy2848048
Copy link
Collaborator

@psy2848048 psy2848048 commented Dec 12, 2024

  • Implemented query methods
  • Please review the commit 387167f .
    • Whole XrplClient.swift
    • BaseMethod.swift -> Protocol and its BaseRequest
    • Tests

@psy2848048 psy2848048 self-assigned this Dec 12, 2024
@psy2848048 psy2848048 changed the title Feat/query Query method implementations Dec 12, 2024
Comment on lines 438 to 448
do {
let res: R = try await http.request(
httpMethod: http.HttpMethod.post,
url: URL.init(string: self.host)!,
params: data
)!

return res
} catch let err {
throw err
}

Choose a reason for hiding this comment

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

Rather than unwrapping, it might be better to maintain type safety and handle values explicitly using optional binding.

Suggested change
do {
let res: R = try await http.request(
httpMethod: http.HttpMethod.post,
url: URL.init(string: self.host)!,
params: data
)!
return res
} catch let err {
throw err
}
guard let url = URL.init(string: self.host) else {
throw http.APIError.invalidURL
}
do {
guard let res: R = try await http.request(
httpMethod: http.HttpMethod.post,
url: url,
params: data
) else{
// Unexpected data format
throw http.APIError.cannotParseRequest
}
return res;
} catch let err {
throw err
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thx to the commit 2a30e8a, the snippet is simplified as c931798

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.

3 participants