-
Notifications
You must be signed in to change notification settings - Fork 93
[GEN][ZH] Simplify repeated calls to removeLastChar with new truncate and trim methods #1257
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
Changes from 4 commits
f53f178
f1064ea
b82d101
b5c039a
a61b34a
d10b002
f5fc609
38ff97e
c472df8
103fb41
d976040
b3f335d
4569dff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -215,19 +215,18 @@ | |
AsciiString dir1 = TheRecorder->getReplayDir(); | ||
AsciiString dir2 = *filename; | ||
AsciiString wildcard = *filename; | ||
const char* lastSep = dir2.reverseFind('\\'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The approach here assumes that there's not a mix of separator characters in a file name. Is this a plausible possibility? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To avoid such problem you can search for both and then take the address that is larger. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this string contains userinput from commandline, so it can contain |
||
if (lastSep == NULL) | ||
{ | ||
int len = dir2.getLength(); | ||
while (len) | ||
{ | ||
char c = dir2.getCharAt(len-1); | ||
if (c == '/' || c == '\\') | ||
{ | ||
wildcard.set(wildcard.str()+dir2.getLength()); | ||
break; | ||
} | ||
dir2.removeLastChar(); | ||
len--; | ||
} | ||
lastSep = dir2.reverseFind('/'); | ||
} | ||
|
||
if (lastSep != NULL) | ||
{ | ||
// include one extra char so the separator itself gets included | ||
int lenToLastSep = lastSep - dir2.str() + sizeof(char); | ||
dir2.truncateTo(lenToLastSep); | ||
Check failure on line 228 in Core/GameEngine/Source/Common/ReplaySimulation.cpp
|
||
wildcard.set(wildcard.str() + lenToLastSep); | ||
} | ||
|
||
FilenameList files; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -245,16 +245,15 @@ void UnicodeString::trim() | |
{ | ||
// Clip trailing white space from the string. | ||
int len = wcslen(peek()); | ||
for (int index = len-1; index >= 0; index--) | ||
int index = len - 1; | ||
while (index >= 0 && iswspace(getCharAt(index))) | ||
{ | ||
if (iswspace(getCharAt(index))) | ||
{ | ||
removeLastChar(); | ||
} | ||
else | ||
{ | ||
break; | ||
} | ||
--index; | ||
} | ||
|
||
if (index < len - 1) | ||
{ | ||
truncateTo(index + 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this correct? index + 1? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think so; index at this point contains the index of the last character that is not a space. Is this perhaps an early warning sign of confusing naming/description of the function and its parameter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the - + 1 stuff can be simplified by starting looping at index = len and then look at index - 1 to test the whitespace char. |
||
} | ||
} | ||
} | ||
|
@@ -263,15 +262,38 @@ void UnicodeString::trim() | |
|
||
// ----------------------------------------------------- | ||
void UnicodeString::removeLastChar() | ||
{ | ||
truncateBy(1); | ||
} | ||
|
||
// ----------------------------------------------------- | ||
void UnicodeString::truncateBy(const UnsignedInt charCount) | ||
{ | ||
validate(); | ||
if (m_data) | ||
if (m_data && charCount > 0) | ||
{ | ||
int len = wcslen(peek()); | ||
size_t len = wcslen(peek()); | ||
if (len > 0) | ||
{ | ||
ensureUniqueBufferOfSize(len+1, true, NULL, NULL); | ||
peek()[len - 1] = 0; | ||
size_t count = min(charCount, len); | ||
peek()[len - count] = 0; | ||
} | ||
} | ||
validate(); | ||
} | ||
|
||
// ----------------------------------------------------- | ||
void UnicodeString::truncateTo(const UnsignedInt maxLength) | ||
{ | ||
validate(); | ||
if (m_data) | ||
{ | ||
size_t len = wcslen(peek()); | ||
if (len > maxLength) | ||
{ | ||
ensureUniqueBufferOfSize(len + 1, true, NULL, NULL); | ||
peek()[maxLength] = 0; | ||
} | ||
} | ||
validate(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file just has whitespace changes now.