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 file function constness #786

Merged
merged 1 commit into from
Feb 10, 2024
Merged

Conversation

xezon
Copy link
Contributor

@xezon xezon commented Nov 9, 2022

Adds const to File member functions, but not yet the virtual functions.

@xezon xezon added the fixing label Nov 9, 2022
@xezon
Copy link
Contributor Author

xezon commented Nov 14, 2022

Looks like your approval is yet non consequential :D

@feliwir
Copy link
Contributor

feliwir commented Nov 14, 2022

Looks like your approval is yet non consequential :D

i thought approving twice can't hurt :) It asked me to add my review again:
image

@jonwil
Copy link
Contributor

jonwil commented Nov 15, 2022

I have evidence that these were not const in the original. Is there a reason to deviate from that?

@xezon
Copy link
Contributor Author

xezon commented Nov 15, 2022

Yes. const correctness is important.

@xezon
Copy link
Contributor Author

xezon commented Feb 10, 2024

To elaborate on const correctness, here is article:
https://isocpp.org/wiki/faq/const-correctness

In a nutshell, anything that is meant to be immutable should be marked const, most importantly on

  • immutable global variables
  • immutable public struct members
  • immutable private class members
  • immutable member functions, that promise to not change state of callee
  • immutable function arguments

const does not change the runtime.

@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (dbfc5bc) 2.53% compared to head (7e4ebc8) 2.53%.

Files Patch % Lines
src/game/common/system/file.h 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #786   +/-   ##
========================================
  Coverage     2.53%    2.53%           
========================================
  Files          949      949           
  Lines       110293   110293           
  Branches     18881    18881           
========================================
  Hits          2800     2800           
  Misses      107089   107089           
  Partials       404      404           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xezon xezon merged commit 7eb9ef8 into TheAssemblyArmada:develop Feb 10, 2024
7 checks passed
@xezon xezon deleted the fix-file-const branch February 10, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants