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

Remove two unnecessary null pointer checks for the variable “m_RPCWaitDlg” #5378

Closed
elfring opened this issue Sep 26, 2023 · 10 comments
Closed

Comments

@elfring
Copy link

elfring commented Sep 26, 2023

👀 The member function “CMainDocument::RequestRPC” contains the statement “m_RPCWaitDlg = new AsyncRPCDlg();”.
Redundant checks are still applied (despite of this implementation detail).
💭 Thus I suggest to remove them.

@AenBleidd
Copy link
Member

You can go ahead and create a PR with the fix 😉

@elfring
Copy link
Author

elfring commented Sep 26, 2023

@davidpanderson
Copy link
Contributor

If we're going to clean up the logic here let's finish the job,
and create the dialog only if it's actually used.

@elfring
Copy link
Author

elfring commented Sep 26, 2023

…, and create the dialog only if it's actually used.

Will further source code adjustments belong to other issues? 🤔

@AenBleidd
Copy link
Member

@elfring, this particular request belongs to the same PR: #5379.

@elfring
Copy link
Author

elfring commented Sep 26, 2023

@davidpanderson did not add his comment as a direct contribution to my pull request for this issue.

@AenBleidd
Copy link
Member

@elfring, ok, if you want, I can say next way: request of @davidpanderson belongs to this particular issue (#5378) and the corresponding PR (#5379)

@elfring
Copy link
Author

elfring commented Sep 26, 2023

💭 I would prefer a clearer separation between remaining open issues for the affected software components.

@AenBleidd
Copy link
Member

@elfring, you're the author of this issue. It was a request from the maintainer to improve your PR. This improvement belongs to the same component and to the same function that is touched by your PR.
Please, take into the consideration the comment from the maintainer and improve your PR.
Thank you in advance

@AenBleidd
Copy link
Member

Closing this as 'not needed'. Please read more detailed explanation here: #5379 (comment)

@AenBleidd AenBleidd closed this as not planned Won't fix, can't repro, duplicate, stale Sep 28, 2023
@github-project-automation github-project-automation bot moved this from Backlog to Done in Client/Manager Sep 28, 2023
@AenBleidd AenBleidd removed the T: Task label Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants