- A wstring as a starting point
A wstring as a starting point
- A maximal length if padding occurs
A maximal length if padding occurs
- A char to repeatedly add after the string for padding
A char to repeatedly add after the string for padding
- A prefix and a suffix that have to enclose the string only if they fit (both or just the first or none*) inside the maximal length.
A prefix and a suffix that have to enclose the string only if they fit (both or just the first or none*) inside the maximal length.
(*) In my opinion, a policy "both or none" is more consistent.
(*) In my opinion, a policy "both or none" is more consistent.
- A wstring as a starting point
- A maximal length if padding occurs
- A char to repeatedly add after the string for padding
- A prefix and a suffix that have to enclose the string only if they fit (both or just the first or none*) inside the maximal length.
(*) In my opinion, a policy "both or none" is more consistent.
A wstring as a starting point
A maximal length if padding occurs
A char to repeatedly add after the string for padding
A prefix and a suffix that have to enclose the string only if they fit (both or just the first or none*) inside the maximal length.
(*) In my opinion, a policy "both or none" is more consistent.
If I understand your starting postulate, the user give:
- A wstring as a starting point
- A maximal length if padding occurs
- A char to repeatedly add after the string for padding
- A prefix and a suffix that have to enclose the string only if they fit (both or just the first or none*) inside the maximal length.
(*) In my opinion, a policy "both or none" is more consistent.
Review
static std::wstring fillWString
I don't see advantage to declare your function as static
.
- If it's a static method from a class, since you "extracted" it from the class for review, just remove the keyword.
- If it's to hide the function from other translation units, just replace it by an anonymous namespace.
- Maybe you added it for another reason, if so, tell me.
Next, for the function signature
const std::wstring &stringToFill,
size_t fillLength,
const wchar_t fillChar = L' ',
const std::wstring &initialFill = L"",
const std::wstring &endFill = L""
As said by @Deduplicator, you should use wstring_view
instead of const wstring&
this will reduce number of allocations and help you to reduce overhead. Your const-correctness is good, just apply it to the size_t
to stay consistent and allow optimizations.
At this point, we can already see another problem: your naming convention. You wrote 26 times the word "fill" for only 16 lines of code (i dont count curly braces nor the empty lines). That pollute the reading since we already know from the function name that's about filling a string.
const size_t originalStringLength = stringToFill.size();
if (fillLength <= originalStringLength) {
return stringToFill;
}
std::wstring result(stringToFill);
result.resize(fillLength, fillChar);
You return the original string if at least as long as the given length.
std::wstring result(stringToFill);
result.resize(fillLength, fillChar);
if (originalStringLength + initialFill.size() <= fillLength) {
result.replace(originalStringLength, initialFill.size(), initialFill, 0u);
if (originalStringLength + initialFill.size() + endFill.size() <= fillLength) {
result.replace(fillLength - endFill.size(), endFill.size(), endFill, 0u);
}
}
You allocate the final size, but after put the filling char in each byte of padding, if needed, you change again bytes where prefix and suffix occurs. Try to avoid overwriting when you can. Furthermore, you did many times same computation, caching is a option.
Here you can see also how too long names make the reading more difficult.
Proposal (using policy "both or none")
#include <iostream>
#include <string>
#include <string_view>
std::wstring fillString(
const std::wstring_view source,
const size_t length,
const wchar_t filler = L' ',
const std::wstring_view prefix = {},
const std::wstring_view suffix = {}
) {
if (length <= source.size()) {
return std::wstring{source};
}
std::wstring result;
result.reserve(length);
result = source;
if (prefix.size() + result.size() + suffix.size() <= length ) {
result.append(prefix);
result.append(length - result.size() - suffix.size(), filler);
result.append(suffix);
}
else {
result.append(length - result.size(), filler);
}
return result;
}