OleksandrKvl / sbepp

C++ implementation of the FIX Simple Binary Encoding
https://oleksandrkvl.github.io/sbepp/
MIT License
42 stars 4 forks source link

Missing `operator->` in iterators #28

Closed ujos closed 6 months ago

ujos commented 6 months ago

Could you please add operator-> to the sbepp::detail::forward_iterator

OleksandrKvl commented 6 months ago

It's not missing, it's just not possible to have. All iterators in sbepp are proxy ones, their value_type objects don't exist in memory, they are created on the fly and returned by-value by operator*, there's no pointer to return in operator->. For this reason, C++20 doesn't require iterators to have operator->.

P.S. I strongly recommend you to always provide real case based motivation for feature requests. Something can be considered "missing" not just because it's absent but because it's actually needed and existing functionality is not enough.

ujos commented 6 months ago

P.S. I strongly recommend you to always provide real case based motivation for feature requests. Something can be considered "missing" not just because it's absent but because it's actually needed and existing functionality is not enough.

I'm working on PoC for the communication layer between two components in our application. I have to choose which message format to choose. Once chosen we may use it to replace the messaging protocol for the rest of services in our eco-system.

Our target platform is Linux gcc7.3 (C++17). We are moving to gcc8 in two months. Most developers use VS2022 as a desktop IDE.

There are a few options for now:

I'm saying that to you so you know, it is not that I just have a lot of time to fool around. Whatever I say/write here is based on my personal experience while I'm learning the sbepp. All issues reported are based on real experience.

While working on PoC I encountered a couple of issues:

auto exchanges = m.exchanges(); sbepp::fill_group_header(exchanges, std::size(exchangesArray));

auto exchangesIt = exchanges.begin(); for(auto exch: exchangesArray) { set((*exchangesIt).name(), exch); // <<<< HERE

exchangesIt++;

}



Me personally I like SBE protocol, so I would rather fight hard before I switch to the Google proto-buffers. I'm really looking forward to continue using this library. Please don't be mad on me if I do not provide the details. Sometimes I just think it is obvious and it does not worth to explain.
OleksandrKvl commented 6 months ago

First of all, you don't need C++20 to use iterators without operator->, I don't know any STL function that relies on it. Thanks for the code sample, now it's much easier to reason about. As I said, it's just not possible to have operator-> but I don't really see a problem here. set((*exchangesIt).name(), exch) is not much verbose than theoretical set(exchangesIt->name(), exch). You can instead iterate over exchanges in a range-for-loop and over exchangesArray manually via iterator:

constexpr auto exchangesArray[] = { "NYSE", "CME", "CBOE" };

auto exchanges = m.exchanges();
sbepp::fill_group_header(exchanges, std::size(exchangesArray));

auto it = std::begin(exchangesArray);
for(const auto entry : exchanges){
    set(entry.name(), *it);
    ++it;
}

You can even iterate over both of them using index:

constexpr auto exchangesArray[] = { "NYSE", "CME", "CBOE" };

auto exchanges = m.exchanges();
sbepp::fill_group_header(exchanges, std::size(exchangesArray));

for(std::size_t i =0; i != exchanges.size(); i++){
    set(exchanges[i].name(), exchangesArray[i]);
}

I'd say that the real problem here is that you're using manual loop and multiple layers of abstraction got mixed. What you really need is to join two views iterate over this joined view and assign the value. It's easy to achieve using ranges or just a handwritten function.

ujos commented 6 months ago

I see, thank you very much for the explanation.

BTW, I've found the following. You do not have to implement it. JFYI.

The operator->() is a bit of an odd-ball: although it can return a non-pointer type, the resulting type would need to overload the operator->(), too!

struct Y {
   int y;
   Y* operator->() { return this; }
};

struct X{
   Y operator->() { return {}; }
};

int main()
{
   auto x = X{};
   auto f = x->y;
}

Source: https://stackoverflow.com/questions/20688066/result-of-operator-yields-non-pointer-result

OleksandrKvl commented 6 months ago

That's actually interesting and seems that it's already used by other libraries for this exact purpose. Will try to implement it so reopening the issue.