feat: introduces fixed string type#42
Conversation
Bumps [requests](https://github.com/psf/requests) from 2.32.5 to 2.33.0. - [Release notes](https://github.com/psf/requests/releases) - [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md) - [Commits](psf/requests@v2.32.5...v2.33.0) --- updated-dependencies: - dependency-name: requests dependency-version: 2.33.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.5.0 to 2.6.3. - [Release notes](https://github.com/urllib3/urllib3/releases) - [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst) - [Commits](urllib3/urllib3@2.5.0...2.6.3) --- updated-dependencies: - dependency-name: urllib3 dependency-version: 2.6.3 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Florian Sattler <florian.sattler@airbus.com>
resolves SEN-1606
resolves SENDESK-106
resolves SEN-1565
resolves SEN-1603
Query params parsing and routing to registered handlers. Resolves SEN-1592
resolves SEN-1578
resolves SEN-1552
resolves SEN-1624
resolves SEN-1564
We need to ensure that the runner time is initialized before we publish objects, as during the process we could emit events that, otherwise, get a wrong timestamp. resolves SEN-1640
… the kernel api **BREAKING CHANGE**: The kernel logger configuration will now only affect any other component's logger if the logger is created using the getOrCreateLogger method specified in the description below. The purpose is to keep spdlog statically linked while ensuring packages can directly access the spdlog registry of the sen::kernel. Note: spdlog is now a public dependency of the sen kernel. resolves SEN-1632
During install we require the resources to be present, hence we need to put them in the export set.
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.6.3 to 2.7.0. - [Release notes](https://github.com/urllib3/urllib3/releases) - [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst) - [Commits](urllib3/urllib3@2.6.3...2.7.0) --- updated-dependencies: - dependency-name: urllib3 dependency-version: 2.7.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
#51) Bumps [idna](https://github.com/kjd/idna) from 3.11 to 3.15. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.md) - [Commits](kjd/idna@v3.11...v3.15) --- updated-dependencies: - dependency-name: idna dependency-version: '3.15' dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
|
||
| // NOLINTBEGIN(readability-identifier-naming) | ||
|
|
||
| template <typename CharT, size_t maxCapacity, typename Traits = std::char_traits<CharT>> |
There was a problem hiding this comment.
I would suggest to add some brief documentation about the class, why/when to use it, and how it relates to std::string.
| explicit constexpr FixedStringBase(std::string_view s) { assignRange(util::makeRange(s.begin(), s.end())); }; | ||
|
|
||
| //------------------------------------------------------------------------------------------------------------------// | ||
| // Basic member functions |
There was a problem hiding this comment.
minor: the separators for the "sections" in the class are not consistent. I would suggest to use what we already do in other clases (and partially in this one): use the visibility tags with a comment. For example:
class A
{
public: // Type definitions
// ...
public: // Basic member functions
// ...
public: // Element access
// ...There was a problem hiding this comment.
We use similar separators in other places to (just with another underline) and I did not see any inconsistencies in the class.
I basically used the same grouping as with std::string from cppref, if you think it's better readable with additional public statement and a small comment right of it, I can rework that.
| checkIdxInRange(idx); | ||
| SEN_DEBUG_ASSERT(idx + std::distance(charRange.begin(), charRange.end()) < maxCapacity + 1); | ||
|
|
||
| for (CharT c: charRange) |
There was a problem hiding this comment.
would memcpy be more performant?
There was a problem hiding this comment.
Hmm, depends. A bit faster yes, the problem currently is just that (at least by the general interface), we cannot assume contiguous memory. The point currently is, some interfaces get InputIterators which do not guarantee really that the memory behind it is contiguous. I pers. don't really care, as most string usages are normally always contiguous but if some one would do something like this:
std::list<char> myLetters;
FixedString<6> str;
str.append(myLetters.begin(), myLetters.end()); // *boom
Another alternative could also be, to move the idx check out, so that the optimizer can better optimize the loop. But in the end it's a hard loop to optimize as the bounds are not always known.
| assignRange(util::makeRange(first, last)); | ||
| return *this; | ||
| } | ||
| FixedStringBase& assign(std::initializer_list<CharT> initList) |
There was a problem hiding this comment.
not related to this function in particular: we aren't documenting the functions, and my guess is that this is because we are following the semantics of std::string. Should we perhaps use \copydoc std::string::<function>?
There was a problem hiding this comment.
Yes, all public functions are a 1:1 correspondents with name and parameters to std::string/std::string_view.
I mean, we can copy all the docs from cppreference to here but I'm not sure if it would not just be enough to look at cpp ref std::string/_view if one wants the behavior/docu. I'm not aware of an automated way of pulling in the docu, as std::string in the std lib normally does also not ship docs, so doxygen linking that to it will not work.
EnriqueParodi
left a comment
EnriqueParodi
left a comment
There was a problem hiding this comment.
very nice! just left some minor comments.
2 participants
No description provided.