From 99adc7338360eb5723b3325e5829b417e4d3cf08 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 22 Mar 2017 18:07:23 -0400 Subject: [PATCH 1/7] IniFile: Make Section constructor explicit --- Source/Core/Common/IniFile.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Core/Common/IniFile.h b/Source/Core/Common/IniFile.h index e2021261b1..e8902f468a 100644 --- a/Source/Core/Common/IniFile.h +++ b/Source/Core/Common/IniFile.h @@ -30,7 +30,7 @@ public: public: Section() {} - Section(const std::string& _name) : name(_name) {} + explicit Section(const std::string& name_) : name(name_) {} bool Exists(const std::string& key) const; bool Delete(const std::string& key); From 46d74a7760e58ec36d59ab525ced11d884bcb267 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 22 Mar 2017 18:19:53 -0400 Subject: [PATCH 2/7] IniFile: Make Section's string constructor instances take strings by value As the name is immediately stored into a class member, a move here is a better choice. This also moves the constructor implementations into the cpp file to avoid an otherwise unnecessary inclusion in the header. This is also likely a better choice as Section contains several non-trivial members, so this would avoid potentially inlining a bunch of setup and teardown code related to them as a side-benefit. --- Source/Core/Common/IniFile.cpp | 6 ++++++ Source/Core/Common/IniFile.h | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index 50752aabc1..cf2c89e0f3 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -39,6 +39,12 @@ void IniFile::ParseLine(const std::string& line, std::string* keyOut, std::strin const std::string& IniFile::NULL_STRING = ""; +IniFile::Section::Section() = default; + +IniFile::Section::Section(std::string name_) : name{std::move(name_)} +{ +} + void IniFile::Section::Set(const std::string& key, const std::string& newValue) { auto it = values.find(key); diff --git a/Source/Core/Common/IniFile.h b/Source/Core/Common/IniFile.h index e8902f468a..72f2317a91 100644 --- a/Source/Core/Common/IniFile.h +++ b/Source/Core/Common/IniFile.h @@ -29,8 +29,8 @@ public: friend class IniFile; public: - Section() {} - explicit Section(const std::string& name_) : name(name_) {} + Section(); + explicit Section(std::string name_); bool Exists(const std::string& key) const; bool Delete(const std::string& key); From d8998b63927c63b120efd7ba63dbe62e491305b0 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 22 Mar 2017 18:49:11 -0400 Subject: [PATCH 3/7] IniFile: Provide an rvalue reference overload for SetLines Allows moving in vectors instead of performing an unnecessary copy. --- Source/Core/Common/IniFile.cpp | 11 +++++++++++ Source/Core/Common/IniFile.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index cf2c89e0f3..065d7e9b3c 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -267,6 +267,11 @@ void IniFile::Section::SetLines(const std::vector& lines) m_lines = lines; } +void IniFile::Section::SetLines(std::vector&& lines) +{ + m_lines = std::move(lines); +} + bool IniFile::Section::GetLines(std::vector* lines, const bool remove_comments) const { for (std::string line : m_lines) @@ -352,6 +357,12 @@ void IniFile::SetLines(const std::string& sectionName, const std::vectorSetLines(lines); } +void IniFile::SetLines(const std::string& section_name, std::vector&& lines) +{ + Section* section = GetOrCreateSection(section_name); + section->SetLines(std::move(lines)); +} + bool IniFile::DeleteKey(const std::string& sectionName, const std::string& key) { Section* section = GetSection(sectionName); diff --git a/Source/Core/Common/IniFile.h b/Source/Core/Common/IniFile.h index 72f2317a91..faff71164f 100644 --- a/Source/Core/Common/IniFile.h +++ b/Source/Core/Common/IniFile.h @@ -67,6 +67,7 @@ public: bool Get(const std::string& key, std::vector* values) const; void SetLines(const std::vector& lines); + void SetLines(std::vector&& lines); bool GetLines(std::vector* lines, const bool remove_comments = true) const; bool operator<(const Section& other) const { return name < other.name; } @@ -124,6 +125,7 @@ public: bool GetKeys(const std::string& sectionName, std::vector* keys) const; void SetLines(const std::string& sectionName, const std::vector& lines); + void SetLines(const std::string& section_name, std::vector&& lines); bool GetLines(const std::string& sectionName, std::vector* lines, const bool remove_comments = true) const; From dbdf693c812a8f57a97023e8782e928f46b1b1eb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 22 Mar 2017 19:03:14 -0400 Subject: [PATCH 4/7] IniFile: Use character literals instead of string literals where applicable Character overloads are generally better overall (range checks aren't necessary, etc). --- Source/Core/Common/IniFile.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index 065d7e9b3c..1fd0f7c343 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -23,7 +23,7 @@ void IniFile::ParseLine(const std::string& line, std::string* keyOut, std::strin if (line[0] == '#') return; - size_t firstEquals = line.find("=", 0); + size_t firstEquals = line.find('='); if (firstEquals != std::string::npos) { @@ -445,7 +445,7 @@ bool IniFile::Load(const std::string& filename, bool keep_current_data) { if (line[0] == '[') { - size_t endpos = line.find("]"); + size_t endpos = line.find(']'); if (endpos != std::string::npos) { @@ -492,7 +492,7 @@ bool IniFile::Save(const std::string& filename) for (const Section& section : sections) { if (section.keys_order.size() != 0 || section.m_lines.size() != 0) - out << "[" << section.name << "]" << std::endl; + out << '[' << section.name << ']' << std::endl; if (section.keys_order.size() == 0) { From b92871111a400cb8cf834db59366400ed9568960 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 22 Mar 2017 19:09:22 -0400 Subject: [PATCH 5/7] IniFile: std::move a std::string in GetLines Also gets rid of an unnecessary string copy. --- Source/Core/Common/IniFile.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index 1fd0f7c343..c58b3cf50e 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -274,13 +274,13 @@ void IniFile::Section::SetLines(std::vector&& lines) bool IniFile::Section::GetLines(std::vector* lines, const bool remove_comments) const { - for (std::string line : m_lines) + for (const std::string& line : m_lines) { - line = StripSpaces(line); + std::string stripped_line = StripSpaces(line); if (remove_comments) { - size_t commentPos = line.find('#'); + size_t commentPos = stripped_line.find('#'); if (commentPos == 0) { continue; @@ -288,11 +288,11 @@ bool IniFile::Section::GetLines(std::vector* lines, const bool remo if (commentPos != std::string::npos) { - line = StripSpaces(line.substr(0, commentPos)); + stripped_line = StripSpaces(stripped_line.substr(0, commentPos)); } } - lines->push_back(line); + lines->push_back(std::move(stripped_line)); } return true; From 35959bdaf9090b993a24d22484b43da9c7ff2fb0 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 22 Mar 2017 19:30:15 -0400 Subject: [PATCH 6/7] IniFile: Replace string joining code with JoinString --- Source/Core/Common/IniFile.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index c58b3cf50e..8877d0f61d 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -68,15 +68,7 @@ void IniFile::Section::Set(const std::string& key, const std::string& newValue, void IniFile::Section::Set(const std::string& key, const std::vector& newValues) { - std::string temp; - // Join the strings with , - for (const std::string& value : newValues) - { - temp = value + ","; - } - // remove last , - temp.resize(temp.length() - 1); - Set(key, temp); + Set(key, JoinStrings(newValues, ",")); } void IniFile::Section::Set(const std::string& key, u32 newValue) From 29ca22905b76eb6640128afa8a19d76290774d33 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 22 Mar 2017 19:32:07 -0400 Subject: [PATCH 7/7] IniFile: Replace a character erase with pop_back() Same thing, more straightforward. --- Source/Core/Common/IniFile.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/Core/Common/IniFile.cpp b/Source/Core/Common/IniFile.cpp index 8877d0f61d..1543d0f96f 100644 --- a/Source/Core/Common/IniFile.cpp +++ b/Source/Core/Common/IniFile.cpp @@ -427,9 +427,9 @@ bool IniFile::Load(const std::string& filename, bool keep_current_data) #ifndef _WIN32 // Check for CRLF eol and convert it to LF - if (!line.empty() && line.at(line.size() - 1) == '\r') + if (!line.empty() && line.back() == '\r') { - line.erase(line.size() - 1); + line.pop_back(); } #endif