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

Take std::string_view instead of const std::string& in most places #105

Open
Ortham opened this issue Apr 1, 2025 · 0 comments
Open

Take std::string_view instead of const std::string& in most places #105

Ortham opened this issue Apr 1, 2025 · 0 comments
Assignees

Comments

@Ortham
Copy link
Member

Ortham commented Apr 1, 2025

In some cases a string reference is needed (e.g. when checking if a string is in a map) or a null-terminated string is needed (e.g. when calling C APIs), so a const std::string& makes more sense, but otherwise it can mean an unnecessary copy of the string to create a std::string if the caller doesn't already have one.

I noticed this recently when writing the C++ wrapper for libloot-rs as its CXX wrapper returns many strings using its own equivalent of std::string_view, so I was having to create copies of the strings just to pass them into functions where they may just be read or may be copied again.

This impacts:

  • The ConditionalMetadata constructor
  • The File constructor
  • The Filename constructor
  • The Group constructor
  • The Location constructor
  • The MessageContent constructor
  • The Message constructor
  • The PluginCleaningData constructor
  • The PluginMetadata constructor
  • The Tag constructor
  • The Vertex constructor
  • PluginMetadata::SetGroup()
  • PluginMetadata::NameMatches()
  • SelectMessageContent()
  • DatabaseInterface::GetGroupsPath()
  • DatabaseInterface::GetPluginMetadata()
  • DatabaseInterface::GetPluginUserMetadata()
  • DatabaseInterface::DiscardPluginUserMetadata()
  • GameInterface::GetPlugin()
  • GameInterface::IsPluginActive()

Since there's an implicit conversion for std::string to std::string_view, this shouldn't require any consumer changes. It is a breaking change though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant