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

Not used string_traits::subject_type #52

Closed
01e9 opened this issue Jan 3, 2018 · 3 comments
Closed

Not used string_traits::subject_type #52

01e9 opened this issue Jan 3, 2018 · 3 comments

Comments

@01e9
Copy link
Contributor

01e9 commented Jan 3, 2018

using subject_type = T; \

I think it should be removed because it confuses those who specialize their types for string_traits. It raises a question: Should I add that alias in my specialization or not?

In the bellow specializations it is not defined

template<> struct PQXX_LIBEXPORT string_traits<const char *>
{
static constexpr const char *name() noexcept { return "const char *"; }
static constexpr bool has_null() noexcept { return true; }
static bool is_null(const char *t) { return !t; }
static const char *null() { return nullptr; }
static void from_string(const char Str[], const char *&Obj) { Obj = Str; }
static std::string to_string(const char *Obj) { return Obj; }
};

@jtv
Copy link
Owner

jtv commented Jan 3, 2018

It's a good point... It's not actually part of the traits API, but it does factor out some repetition of the type, which may help maintenance, and helps hide uninteresting differences between the specialisations.

What if I just made subject_type private?

@01e9
Copy link
Contributor Author

01e9 commented Jan 3, 2018

it does factor out some repetition of the type, which may help maintenance, and helps hide uninteresting differences between the specialisations.

I know that type aliases are handy in templates, but I did not get this point, especially if that alias is not used anywhere (at least in headers).

What if I just made subject_type private?

It will be removed from the focus. I am ok with this solution.

@01e9 01e9 closed this as completed Jan 4, 2018
@jtv
Copy link
Owner

jtv commented Jan 4, 2018

Actually, you were right. In this instance (and one other) it could be completely removed. After a decade or so writing a codebase like this, it becomes hard to see these things. :-)

jtv added a commit that referenced this issue Jan 4, 2018
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