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

Codechange: Use member initalisers #11304

Closed
wants to merge 1 commit into from

Conversation

Berbe
Copy link
Contributor

@Berbe Berbe commented Sep 15, 2023

Motivation / Problem

Looking at the code, I saw several occasions in which member initialisation could be done directly.

Description

Use member initialisers

Limitations

Definitely partial changes, but maybe a step in a good direction?

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR touches english.txt or translations? Check the guidelines
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, game_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@@ -207,12 +207,8 @@ struct PacketWriter : SaveFilter {
* Create a new socket for the server side of the game connection.
* @param s The socket to connect with.
*/
ServerNetworkGameSocketHandler::ServerNetworkGameSocketHandler(SOCKET s) : NetworkGameSocketHandler(s)
ServerNetworkGameSocketHandler::ServerNetworkGameSocketHandler(SOCKET s, NetworkAddress client_address) : NetworkGameSocketHandler(s, _network_client_id++), status(STATUS_INACTIVE), receive_limit(_settings_client.network.bytes_per_frame_burst), client_address(client_address)
Copy link
Member

Choose a reason for hiding this comment

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

Lol, two years later, same comment: please don't do a ++ in a ctor initializer like this. That is going to be overlooked and will bite us in the ass :) ctor member initializer should be without side-effects. Side-effects go in the body.

@2TallTyler
Copy link
Member

Also, this PR should target master, not a release branch.

@2TallTyler 2TallTyler added the work: needs rebase This Pull Request needs a rebase label Nov 9, 2023
@2TallTyler
Copy link
Member

This seems like change for the sake of change, and hasn't been touched since being reviewed. I am closing it, but as always if anyone disagrees feel free to re-open.

@2TallTyler 2TallTyler closed this Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work: needs rebase This Pull Request needs a rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants