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 wallet instance checks in the jsonrpc_status daemon method #1267

Closed
wants to merge 1 commit into from

Conversation

akinwale
Copy link
Contributor

No description provided.

wallet_is_encrypted = has_wallet_with_network and self.session.wallet.wallet and \
self.session.wallet.wallet.use_encryption
headers_progress_percent = self.session.wallet.get_headers_progress_percent() if has_wallet else 0
headers_progress_percent = 0 if not has_wallet else self.session.wallet.get_headers_progress_percent()
Copy link
Member

Choose a reason for hiding this comment

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

In Python you can have a function followed by an if ... and the function won't be called unless the condition is true. Here is an example:

>>> def alternative():
...   print('alternative called!')
... 
>>> alternative() if False else 'nope!'
'nope!'
>>> 

It's difficult to tell what is actually changed above, logically. Can you undo all of the swapped conditionals and just leave the line that fixed the original problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eukreign There's a discussion thread about these changes. I will send you a link.

Copy link
Member

Choose a reason for hiding this comment

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

I read the chat. Maybe the confusion is from the comment about reading from left to right? In which case that's referring to attribute access and not the order in which inline if statements are evaluated. See my code example above which shows that alternative() does not get called.

@akinwale
Copy link
Contributor Author

No longer required.

@akinwale akinwale closed this Jun 28, 2018
@jackrobison jackrobison deleted the fix-has-wallet-check branch January 30, 2019 16:22
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