Thread: How do I create a filename with multiple variable types

[Edit]This all came off a little rude; I apologize for that. That was not my intent.[/Edit]

What does that <anonymous > mean, though ?

In this particular case it is implying that some details of the standard C++ library provided by "MSVC" has some details implemented in a nested anonymous "namespace" probably for the sake of implementation clarity.

Code:

namespace test
{
namespace
{
// this is anonymous... GUY!
}
}

But the WHY is what I don't understand.

O_o

It doesn't work for several reasons, but most of them all relate to the same issue; you are attempting to transform a "r-value" to a "l-value". "MSVC" is really not strict enough with constants and references.

You are also using `char' for a temporary and member variable instead of the correct type. You aren't allowing for the full "weight" of the template. (Using custom traits breaks the code.)

The worst offense however is using `assert' to check for a condition that must always be performed otherwise the mechanism simply "does not do what is says on the lid". Please, if you don't know exactly how to use `assert', do not use it all. It is better to always perform a check that isn't strictly necessary than to not perform a check that is always necessary.

All that said, the approach is wrong. There is no reason to transform the stream (by wrapping it in another component).

The implementation manasij7479 used isn't correct or complete, but the simpler approach is better.

Soma

Originally Posted by phantomotap

It doesn't work for several reasons, but most of them all relate to the same issue; you are attempting to transform a "r-value" to a "l-value". "MSVC" is really not strict enough with constants and references.

Ah, of course.
*smacks forehead*

You are also using `char' for a temporary and member variable instead of the correct type. You aren't allowing for the full "weight" of the template. (Using custom traits breaks the code.)

Well pointed out. That is a bug which I have fixed.

The worst offense however is using `assert' to check for a condition that must always be performed otherwise the mechanism simply "does not do what is says on the lid". Please, if you don't know exactly how to use `assert', do not use it all. It is better to always perform a check that isn't strictly necessary than to not perform a check that is always necessary.

In my opinion, it is always better to do something than nothing at all.
We have three choices: ignore it, assert it or throw an exception. Which one is best? I don't know. Therefore, I put an assert there to remind me that if I did something wrong, I would become reminded.

All that said, the approach is wrong. There is no reason to transform the stream (by wrapping it in another component).

The implementation manasij7479 used isn't correct or complete, but the simpler approach is better.

I'm almost afraid to ask what a "better" solution that doesn't involve explicitly specifying template parameters would be.
Also, this isn't about simpler. If it was simple, I could just use a dummy temp char variable to read into. This is just about implementing laserlight's solution in any way or form for fun.

^_^
So I made another attempt to fix the problems (and this time it compiles with GCC; woohoo!):

Code:

#include <sstream>
#include <iostream>
#include <vector>
#include <assert.h>

#define BASIC_ISTREAM std::basic_istream<StreamT, std::char_traits<StreamT>>

template<typename StreamT>
class XLiteral
{
public:
	XLiteral(StreamT c): m_c(c) {}
	void SetStream(BASIC_ISTREAM& stream) { m_stream = &stream; }
	template<typename T> XLiteral& operator >> (T& rhs) { assert(m_stream); *m_stream >> rhs; return *this; }
	XLiteral& operator >> (XLiteral&& rhs);
	StreamT GetChar() const { return m_c; }

protected:
	BASIC_ISTREAM* m_stream;
	StreamT m_c;
};

template<typename StreamT>
void ExtractLiteral(BASIC_ISTREAM& stream, XLiteral<StreamT>&& literal)
{
	StreamT tmp;
	stream >> tmp;
	// How do we handle the case when the read literal isn't what we specified to the call to literal?
	assert(tmp == literal.GetChar());
	literal.SetStream(stream);
}

template<typename StreamT>
XLiteral<StreamT>& XLiteral<StreamT>::operator >> (XLiteral<StreamT>&& rhs) { ExtractLiteral(*m_stream, std::move(rhs)); return *this; }


template<typename StreamT>
XLiteral<StreamT> & operator >> (BASIC_ISTREAM& lhs, XLiteral<StreamT>&& rhs)
{
	ExtractLiteral(lhs, std::move(rhs));
	return rhs;
}

template<typename StreamT>
XLiteral<StreamT> literal(StreamT c) { return XLiteral<StreamT>(c); }

int main()
{
	int month, day, year;
	std::stringstream ss("31-01-12");

	//ss >> day >> literal('-') >> month >> literal('-');
	ss >> day >> literal('-') >> month >> literal('-') >> year;
}

Since you people seem rather interested, I have attached the file that I uploaded to the then-existing Boost Vault as the proof of concept for my interest check to Boost concerning such an input manipulator. I made this interest check before I approached brewbuck, with the code inspired by an input manipulator implementation in some version of g++'s standard library implementation.

A member of the Boost mailing list pointed out that changing basic_istream itself to overload operator>> could be better than a manipulator if this were ever to be added to the C++ standard: Interest check: constant input matching manipulator.

Note that this is just code to start a discussion: it is not guaranteed to be robust, complete, efficient or even correct; in addition to the disclaimers in the Boost license, I stress that I accept no liability for any damages resulting from the use of the code. You have been warned

EDIT:

Originally Posted by Elysia

In my opinion, it is always better to do something than nothing at all.
We have three choices: ignore it, assert it or throw an exception. Which one is best? I don't know. Therefore, I put an assert there to remind me that if I did something wrong, I would become reminded.

The mistake is that you made a function call in your assertion. If the assertion is disabled, rhs.GetChar() is not called, which looks wrong, don't you agree?

EDIT #2: Oh, but GetChar() is a const member function with no side effects, so I guess that's okay here.

EDIT #3: Come to think of it, the header file is less than 100 lines long, so I might as well post it here for those who would rather not download it:

Code:

#ifndef BOOST_IOMANIP_HPP_
#define BOOST_IOMANIP_HPP_

//          Copyright Eugene Wee 2009.
// Distributed under the Boost Software License, Version 1.0.
//    (See accompanying file LICENSE_1_0.txt or copy at
//          http://www.boost.org/LICENSE_1_0.txt)

/* boost::iomanip::literal is an input stream manipulator that matches input
 * with a specified string or character literal. If the input read matches the
 * literal, it is ignored, otherwise the stream is placed in a failed state.
 * Whitespace is not skipped, hence the std::ws manipulator should be used to
 * skip preceding whitespace, if necessary.
 */

#include <string>
#include <memory>
#include <istream>
#include <ios>

namespace boost
{
namespace iomanip
{
    namespace detail
    {
        // Implementation defined type to implement the manipulator.
        template<typename charT, typename Traits = std::char_traits<charT>,
            typename Allocator = std::allocator<charT> >
        struct token_matcher
        {
            typedef std::basic_string<charT, Traits, Allocator> token_type;
            token_matcher(const token_type& token) : token(token) {}
            token_type token;
        };

        // Extracts and matches input character-wise from the stream.
        template<typename charT, typename Traits, typename Allocator>
        std::basic_istream<charT, Traits>&
        operator>>(std::basic_istream<charT, Traits>& in,
            token_matcher<charT, Traits, Allocator> matcher)
        {
            typename std::basic_string<charT, Traits, Allocator>::iterator
                i = matcher.token.begin(),
                end = matcher.token.end();
            typename std::basic_istream<charT, Traits>::char_type c;
            for (; i != end && in.get(c); ++i)
            {
                if (*i != c)
                {
                    in.clear(std::basic_ios<charT, Traits>::failbit);
                    break;
                }
            }

            return in;
        }
    } // namespace detail

    // Match with literals as standard string objects.
    template<typename charT, typename Traits, typename Allocator>
    inline
    detail::token_matcher<charT, Traits, Allocator>
    literal(const std::basic_string<charT, Traits, Allocator>& token)
    {
        return detail::token_matcher<charT, Traits, Allocator>(token);
    }

    // Match with literals as null terminated strings.
    template<typename charT>
    inline detail::token_matcher<charT> literal(const charT* token)
    {
        return detail::token_matcher<charT>(std::basic_string<charT>(token));
    }

    // Match with literals as individual characters.
    template<typename charT>
    inline detail::token_matcher<charT> literal(charT token)
    {
        return detail::token_matcher<charT>(std::basic_string<charT>(1, token));
    }
} // namespace iomanip
} // namespace boost

#endif

I'm almost afraid to ask what a "better" solution that doesn't involve explicitly specifying template parameters would be.

O_o

It would take you... I'd guess two minutes and 80 characters to simplify the interface for the code manasij7479 posted so that no explicit instantiation would be required and still solves the problem without transforming the stream (by wrapping it).

In my opinion, it is always better to do something than nothing at all.

Which begs the question, why are you doing nothing at all?

Therefore, I put an assert there to remind me that if I did something wrong, I would become reminded.

That is not the purpose of `assert'. The `assert' function is a debugging aid used by implementations to provide context to client code to make the client code easier to debug. The `assert' doesn't exist for release builds; it can't remind you of something in a release build.

Using an `assert' for a "precondition" check is a fair example; let's call that example `void example(int f)'. A "precondition" of `example' states that `f' must never be zero. The debug build notifies client code that an error occurs during debug builds only so that client code can position a check for that same precondition at the best possible location as a convenience to the programmer of client code. (This isn't a necessary component of the implementation; it literally exists only to aid the programmers of client code in getting the use of that implementation correct.) The release build does not perform the check because it is a precondition for which client code assumes responsibility. That precondition check doesn't exist to help the implementation of `example' in any way; the code is simply written against the assumption that `f' is never zero.

The client code is what benefits from the `assert' function. That is an extremely important part of using `assert' correctly. You can't use an `assert' to implement some mechanism because it is not guaranteed to happen.

We have three choices: ignore it, assert it or throw an exception.

*smacks Elysia with newspaper*

No!

This test:

Code:

tmp == literal.GetChar()

must always be performed and an `NDEBUG' `assert' never performs that test.

This is a stream operation we are discussing, a primitive designed to process data and return a formatted result. Such operations are by their very nature going to fail even on full (release) builds. You could argue that what you do when that test fails is up for grabs. You could do the test and fail with `abort' (as `assert' very likely would in a debug build). You could through an exception. You could, should, use `failbit' as that is its purpose. But you can't ignore it; the test must be performed otherwise it simply does not do what it claims.

With an `NDEBUG' `assert' it is exactly equivalent to ignoring one character. No test for proper formatting occurs. That is a clear and obvious violation of the name and its stated purpose.

[Edit]

Oh, but GetChar() is a const member function with no side effects, so I guess that's okay here.

I really wish people would take more than two seconds to read posts other people make.

It is not "okay"; it is absolutely, spectacularly, completely WRONG!

The `assert' and therefore the test is compiled out in a release build.

The code:

Code:

int x;
int y;
if(cin >> x >> literal('+') >> y)
{
    cout << "Yay!";
}

essentially becomes:

Code:

int x;
int y;
char ignored;
if(cin >> x >> ignored >> y)
{
    cout << "Yay!";
}

in a release build.

Does that sound "Okay."?

No! It sounds wrong. It is wrong.

In a release build if I input "1=2" I get a "Yay!" even though that is clearly not what the client code is asking to do. Does that sound "Okay."? Do you want your code to be unaware of an extremely probable user error?

In a debug build if I input "1=2" I get an application abort. Does that sound "Okay."? Do you want your code crashing because of an extremely probable user error?
[/Edit]

Soma

I may agree with you that an assert is not a good thing to use to handle a specific condition. In this case, I chose to not handle the case because I was unsure on how to handle it. So I basically said, "This code requires the precondition that the read char is equal to the one we specified."
But perhaps that isn't enough? In case I do not handle something, perhaps I should simply throw an exception, throw ErrNotImplemented?

Just to be clear, I understand your point. It's just that the code was incomplete because I didn't make a decision on how to handle a specific case. That said, what you say, what laserlight's code does and what mana does and says all sound like a very good idea.

Originally Posted by phantomotap

It is not "okay"; it is absolutely, spectacularly, completely WRONG!

The `assert' and therefore the test is compiled out in a release build.

Having the test compiled out in a release build is okay, insofar as the test is not required as it is expected to always pass. The problem is that the test is required as part of the feature, and is expected to fail in a way that does not constitute a programming error. Unfortunately...

Originally Posted by phantomotap

I really wish people would take more than two seconds to read posts other people make.

Guilty as charged
I assumed that Elysia was just using the assertion as a debugging tool, since any sane programmer does that.

Originally Posted by Elysia

So I basically said, "This code requires the precondition that the read char is equal to the one we specified."

In that case, you don't need the whole business of passing a specific character. You can just assume that the user input is correct, and do the equivalent of a cin.get() that discards the char read, whatever it is.

EDIT:
Furthermore, if you are not going to actually check, then overloading operator>> is a bad idea. It would be better to have an input manipulator that discards n characters read from the stream, i.e., a version of cin.ignore that is a manipulator.

So... going by your information, how would one say "This is an invalid sequence, but I haven't been able to decide which of the four above alternatives this sequence corresponds to"?
Basically, I did not know how to handle it, so I placed a placeholder until I could figure it out. How does one deal with that, then?

Originally Posted by phantomotap

How did you manage to get "the test is not required" and "the test is required" only two sentences apart?

I really wish people would take more than two seconds to read posts other people make.

The part about the test being not required is a general statement about assertions. The part about the test being required is a statement about this particular code that Elysia wrote. This is indicated by the key word "insofar".

Originally Posted by phantomotap

Your stated rationale for both sentences is wrong in any event.

You are mistaken.

Originally Posted by phantomotap

Saying you expect that test to always pass is the same exact thing as saying you always expect input to be perfectly well formed.

I agree, and that is part of my point in post #24 (input is "expected to fail in a way that does not constitute a programming error"). That's why the assertion is wrongly used, regardless of the value of NDEBUG.

Originally Posted by phantomotap

Consuming a literal value provided by the client code is the feature. That is only and exactly what it claims to do. Without the test it does not consume a literal value provided by the client code; it only consumes and discards input. These two things are not even in the same galaxy.

I agree, and that is part of my point in post #24 ("the test is required as part of the feature"). Basically, you repeated my mistake of not reading carefully, except that this time it is not about code, but about prose.

Originally Posted by phantomotap

And so I infer:

Having the test compiled out in a release build is okay<comma> it can be ascertained that the test is not required as it is expected to always pass.

And so I read that as affirming the use of `assert' to catch some bizarre condition that violates that expectation.

O_o

I find it bizarre that you inferred certainty from "to such extent".

Since the next sentence states that the test is expected to fail, maybe it is almost as if I managed to catch you with a "garden path" pair of sentences, heh.

Originally Posted by phantomotap

And how wonderful it is that code isn't so open to interpretation.

Except when it is, like Elysia's "I put an assertion here as a placeholder until further notice" kind of thing

Anyway, is anyone interested in seeing this functionality, whether as input manipulator or as operator>> overloads, added to the C++ standard?