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

added get_block_header issue #262 #263

Merged
merged 1 commit into from Apr 21, 2017

Conversation

oxarbitrage
Copy link
Member

No description provided.

for (const uint32_t& i : block_num) {
auto result = _db.fetch_block_by_number(i);
if (result)
results[i] = *result;
Copy link
Member

@xeroc xeroc Apr 19, 2017

Choose a reason for hiding this comment

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

Please excuse this comment - I am not much a C++ dev - but shouldn't we prefer results.push_back(result) over using pointers?
That way you can also push a null object, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

thank you for the comment @xeroc . you might be right but i used the same code as get_block_header:

optional<block_header> database_api_impl::get_block_header(uint32_t block_num) const
{
   auto result = _db.fetch_block_by_number(block_num);
   if(result)
      return *result;
   return {};
}

adding @vikramrajkumar can probably help decide.

Copy link
Contributor

Choose a reason for hiding this comment

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

results[i] = *result; is correct because results is a map, and the null result is handled in the subsequent line results[i] = {};

A minor suggestion for the future: in this case you can prefer uint32_t instead of uint32_t& in the range-based for loop. Since uint32 i is a small native type, it is more efficient to copy the value for each iteration of the loop.

@vikramrajkumar vikramrajkumar merged commit b50409d into bitshares:master Apr 21, 2017
@oxarbitrage oxarbitrage deleted the issue262_2 branch August 23, 2018 21:10
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

3 participants