Skip to content
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

Separate concepts for reading and writing containers #319

Closed
justend29 opened this issue Jun 10, 2023 · 5 comments
Closed

Separate concepts for reading and writing containers #319

justend29 opened this issue Jun 10, 2023 · 5 comments

Comments

@justend29
Copy link
Collaborator

Although the use of concepts throughout Glaze is marvellous, I've run into a problem where they're overly restrictive.

For example, the map_t concept is applied to the default map reading and map writing. This map_t concept requires map_subscriptable, but this seems to only be required for reading, not writing.

I would like to instead offer separate concepts for maps, and the other types, that are specific to reading or writing. This will allow writing forward ranges, which don't have direct access, like map_subscriptable.

Slightly contrived example:

auto free_ports = {std::pair("tcp", std::views::iota(8000, 8010) | std::views::filter(free_port)),
                   std::pair("udp", std::views::iota(9000, 9010) | std::views::filter(free_port))};
        
glz::write_json(free_ports);

/*
{
  "tcp": [8000, ...],
  "udp": [9031, ...]
}
*/

I'm happy to contribute these changes, with tests of course, but would first like to get feedback to see if I'm misinterpreting anything and get your general feelings about these changes before they are made. I also don't have the time to complete this immediately, so it might be a bit.

@stephenberry
Copy link
Owner

Have you looked at the new glz::obj and glz::arr logging structures? Currently this only supports writing and not reading, but may solve your problem.
I do really like the idea of generally handling initializer lists, so I'll keep this issue alive to handle them.

From the README:

Sometimes you just want to write out JSON structures on the fly as efficiently as possible. Glaze provides tuple-like structures that allow you to stack allocate structures to write out JSON with high speed. These structures are named glz::obj for objects and glz::arr for arrays.

Below is an example of building an object, which also contains an array, and writing it out.

auto obj = glz::obj{"pi", 3.14, "happy", true, "name", "Stephen", "arr", glz::arr{"Hello", "World", 2}};

std::string s{};
glz::write_json(obj, s);
expect(s == R"({"pi":3.14,"happy":true,"name":"Stephen","arr":["Hello","World",2]})");

@justend29
Copy link
Collaborator Author

Thanks for the feedback.

I am aware of these new logging structures, but my example really was contrived. What I was trying to illustrate is there's some arbitrary input/forward range that needs to be written. In my use case, this input range isn't actually static and needs to be transformed into a different format for serialization.

An example more apt may be:

glz::write_json(gather_outside_information() | std::views::transform(information_to_pair))

Separating my use case from these concepts, though, I do believe it's more accurate to divide glaze's concepts into those for reading and those for writing, as the requirements are different, and there's no need to require what's not used. I see that you feel there's a benefit here.

If I'm understanding everything correctly, and correct me if I'm not, it's worthwhile to separate the concepts for reading from writing containers. Does a naming system like writable_map_t & readable_map_t instead of just map_t sound reasonable (following the _t)?

@stephenberry
Copy link
Owner

Thanks for The additional information. I think it makes sense to separate concepts for write and read, writeable_map_t and readable_map_t sound great. I'd accept this change if you made a pull request.

@justend29
Copy link
Collaborator Author

This call seems to avoid the skip_null_members check for the first entry in a map that all the subsequent entries go through.

Is my interpretation of this correct? Is that desired?

stephenberry added a commit that referenced this issue Jul 2, 2023
* add cmake preset file for simpler build+test

* save unchanged common.hpp - clang-format changes

* attempt to separate write from read of mapt_t

* use result of [[nodiscard]] function in test; avoid warning

* gracefully fail testing variant to avoid subproc aborted

* fix variant auto-deducible func w/ split maps

* rm 'value_type' nested type alias requirement for ranges

* refactor to_json(map) for in ranges + first el mistreatment

* require 'range' concept to be input range

* use free functions that are compat. w/ more types

* create some tests for writing map-like ranges

* add basic writing in input ranges as arrays

* replace ranges::empty() w/ internal alternative for older compilers

* disable tests w/ views when views unavailable

* fix circular include

* fix MSVC literal issue

* use glz::sv instead of literal sv

* attempt MSVC fix removing operator""

* serialize pair as object

* correct expected json in write pair test

* provide reading specialization for pair

* uncomment "issue". UT can run, there's just no default printing for maps

* test reading pairs

* read binary pair specialization

* refactor json tests to avoid expected. glz::expected subset of std::expected

* provide deduction guides on test-case structs for apple-clang

* classify pair_t as object when auto-deducing variant

* document breaking std::pair change

* Version 1.3.0 bump

---------

Co-authored-by: Stephen Berry <stephenberry.developer@gmail.com>
@justend29
Copy link
Collaborator Author

Completed: PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants