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

Allow using another string_view implementation #128

Open
pitrou opened this issue Feb 27, 2019 · 7 comments
Open

Allow using another string_view implementation #128

pitrou opened this issue Feb 27, 2019 · 7 comments

Comments

@pitrou
Copy link

pitrou commented Feb 27, 2019

The uri library comes with its own string_view backport. However, I am already using another backport (and of course some people are using C++17 and simply want the stdlib version).

It would be nice to make the uri library configurable in that regard (I'm not sure how though...). Right now I would have to do some invasive patching to get it working for my purposes.

@pitrou
Copy link
Author

pitrou commented Feb 27, 2019

(note a similar issue may hold for the optional backport)

@glynos
Copy link
Member

glynos commented Feb 27, 2019

I don't know what your project requirements are, but if you have access to a C++17 compliant compiler, you can use the WhatWG URL that supersedes this one: https://github.com/cpp-netlib/url

@pitrou
Copy link
Author

pitrou commented Feb 27, 2019

No, we are C++11-compatible unfortunately :-(

@glynos
Copy link
Member

glynos commented Feb 28, 2019

This is going to be difficult to test, as there are going to be many string_view variants, each with tiny differences. Would it be enough to write a simple adapter class around this one, e.g.:

#include <network/uri.hpp>

namespace mynamespace {
class myuri {

  //...

  mystring_view scheme() const noexcept {
    auto sv = url_.scheme();
    return mystring_view(sv.data(), sv.size());
  }

private:
  network::uri uri_;
};
}

@pitrou
Copy link
Author

pitrou commented Feb 28, 2019

That sounds a bit cumbersome IMHO.

Thinking out loud, one possibility would be to have a parametered design, such as:

// basic_uri.hpp
namespace network {
template <class string_view, template<class> class optional>
class basic_uri {
  // regular class definition here
};
}

// uri.hpp
#include "basic_uri.hpp"
#include "optional.hpp"
#include "string_view.hpp"

namespace network {
typedef basic_uri<string_view, optional> uri;
}

Then in our library I could write e.g.:

#include <network/uri/base_uri.hpp>

typedef base_uri<arrow::util::string_view, arrow::util::optional> uri;

It seems like that would work, though I may be missing something...

@glynos
Copy link
Member

glynos commented Mar 1, 2019

Thanks for the suggestion.

First of all, the optional class implemented in this library is not a part of the public interface, so there should be no conflict with interfacing with client code. Secondly, by design, string_view does nothing more than copy a pointer and a integer (to store the size), so there should be almost no run-time cost to using the adapter pattern I recommended above.

Therefore I don't believe there is any benefit to re-defining the uri class to use a parameterized template in this way.

@pitrou
Copy link
Author

pitrou commented Mar 1, 2019

Right, the adapter pattern wouldn't have a runtime cost, just a per-project maintenance cost. There is also the compilation time cost of having two different string_view implementations + an adapter...

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