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

Move string copyBuff from implmentation to string base class #695

Merged
merged 1 commit into from
Jun 15, 2021

Conversation

Joshua-Anderson
Copy link
Contributor

Originating Project/Creator
Affected Component
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n)
Builds Without Errors (y/n)
Unit Tests Pass (y/n)
Documentation Included (y/n)

Change Description

Convert string copyBuff from a virtual function that was implemented by string implementations to a method in the base string class.

Rationale

Removes code duplication of the copyBuff function and reduces the chance of introducing an error when implementing a string class. It also was triggering clang-tidy warnings because calling a virtual function in a constructor is highly discouraged due to the chance of calling an unintended implementation of the virtual function.

@LeStarch
Copy link
Collaborator

LeStarch commented Jun 9, 2021

@timcanham could you look at this, and speak to why this wasn't done previously?

@LeStarch LeStarch requested a review from timcanham June 9, 2021 19:16
@timcanham
Copy link
Collaborator

This is a good change. I think we didn't really notice it because it was mostly hidden in code generation. This will save a bunch of code...

@LeStarch LeStarch merged commit 8d3b1ca into nasa:devel Jun 15, 2021
@Joshua-Anderson Joshua-Anderson deleted the cleanup-str-copybuff branch June 15, 2021 16:38
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