On 11.02.2017 13:52, Chris Vine wrote:
> On Sat, 11 Feb 2017 12:53:01 +0100
> "Alf P. Steinbach" <
alf.p.stein...@gmail.com> wrote:
>> On 11.02.2017 12:25, Chris Vine wrote:
>>>
>>> The only thing that struck me about Alf's implementation is that the
>>> starts_with() function is in the recursion hot path and by calling
>>> std::string::substr() might allocate memory
>>
>> I also compared strings for equality when known that one is leading
>> substring of other, instead of just comparing lengths then.
>>
>> Even for a first working version that was, well, dumb. :)
>
> I think your code was OK on the equality test.
Well, slower than necessary. :o)
> The dictionary lower
> bound has been reset on re-entering recursively_append_words_from(),
> and end_dict_range is always one past the end of the dictionary, so
> 'word' might no longer be a leading substring of *lower. (It occurs
> to me that since 'lower' could be at 'end_dict_range', this should
> probably be checked for before 'lower' is dereferenced on calling
> starts_with().)
Good point, I didn't think of that. That's a bug.
Another bug is that I intended to use a matrix with border but as I can
see it was dimensioned as (n + 1)×(n + 1) instead of (n + 2)×(n + 2).
Ugh. I can believe I did that, but then, I just cobbled something up.
I wonder if Andrey Karpov's static checked (PVS studio) could have any
chance of detecting these bugs?
For the non-dereferencable iterator, instead of extra checking one can
theoretically add a dummy zero-length word, a sentinel value, at the end
of the dictionary and pass iterator to that, instead of beyond, as
end-iterator. But that to some degree breaks the structure of the
design, e.g. the same code can't be handed a dictionary that is without
such a sentinel value, and trying it out I found that it also made for
more complicated `assert`s about the data. So, an extra check it is.
With the bugs fixed the code, minus the Matrix class definition and the
file reading & checking, now looks like this in ¹Expressive C++:
inline $function starts_with(
ref_<const string> prefix,
ref_<const string> s
) -> bool
{
$let prefix_length = length_of( prefix );
if( prefix_length > length_of( s ) ) { return false; }
return s.compare( 0, prefix_length, prefix ) == 0;
}
using Dictionary_iter = vector<string>::const_iterator;
struct Position
{
Index x;
Index y;
};
$procedure recursively_append_words_from(
const Dictionary_iter start_dict_range,
const Dictionary_iter end_dict_range,
ref_<const Matrix<char>> board,
const Position position,
ref_<Matrix<bool>> visited,
ref_<string> word,
ref_<vector<string>> result
)
{
$let original_word_length = word.length();
$let ch = board.item( position.x, position.y );
if( ch == 'q' ) { word += "qu"; } else { word += ch; }
$let it_dict_entry = lower_bound( start_dict_range,
end_dict_range, word );
if( it_dict_entry != end_dict_range and starts_with( word,
*it_dict_entry ) )
{
$let length = word.length();
if( length >= 3 and length == it_dict_entry->length() )
{
result.push_back( word );
}
visited.item( position.x, position.y ) = true;
for( $each dy $in range( -1, +1 ) ) for( $each dx $in
range( -1, +1 ) )
{
Position const new_position = {position.x + dx,
position.y + dy};
if( not visited.item( new_position.x, new_position.y ) )
{
recursively_append_words_from(
it_dict_entry, end_dict_range, board, new_position,
visited, word, result
);
}
}
visited.item( position.x, position.y ) = false;
}
word.resize( original_word_length );
}
$procedure append_words_from(
ref_<const vector<string>> dictionary,
ref_<const Matrix<char>> board,
ref_<const Position> start,
ref_<vector<string>> result
)
{
string word;
$let board_size = board.size();
Matrix<bool> visited{ board_size };
for( $each i $in i_up_to( board_size ) )
{
visited.item( i, 0 ) = true;
visited.item( 0, i ) = true;
visited.item( i, board_size - 1 ) = true;
visited.item( board_size - 1, i ) = true;
}
recursively_append_words_from(
begin( dictionary ), end( dictionary ) - 1, board, start,
visited, word, result
);
}
$procedure cpp_main( ref_<const vector<string>> args )
{
// Using non-macro pure C++ `fail` to get unadorned message in
exception:
$hopefully( n_items_in( args ) == 3 )
or fail( ""s << "Usage: " << args[0] << " DICTIONARY
BUBBLEBOARD" );
vector<string> const dictionary = load_dictionary( args[1] );
Matrix<char> const board = load_bubble_board( args[2] );
assert( is_sorted( $items_of( dictionary ) ) );
$let board_size = board.size(); $let n = board_size - 2;
assert( n > 0 );
vector<string> words;
for( $each y $in i_up_to( n ) ) for( $each x $in i_up_to( n ) )
{
append_words_from( dictionary, board, {x + 1, y + 1}, words );
}
sort( $items_of( words ) );
$let start_of_duplicates = unique( $items_of( words ) );
words.erase( start_of_duplicates, end( words ) );
for( $each word $in words ) { cout << word << "\n"; }
}
Cheers!,
- Alf
Notes:
¹ It's a little language extension library I couldn't resist creating
after the “Fun with macros” posting here in clc++. The macros, supported
by various templates, currently are defined as follows:
# define $e ::progrock::expressive
#
# define $static_assert EXPRESSIVE_STATIC_ASSERT // Single
arg like C++17.
# define $funcname EXPRESSIVE_FUNCNAME // Qualified
__func__.
# define $noreturn EXPRESSIVE_NORETURN // MSVC
lacks [noreturn].
#
# define $function auto
# define $procedure void
#
# define $simple_pure_function constexpr $function
# define $compile_time_value static constexpr
#
# define $an_expected( Type, expr ) \
( \
[&]{ $static_assert(( ::std::is_same<Type,
decltype((expr))>::value )); }, \
expr \
)
# define $as static_cast
# define $let auto const
# define $let_mutable auto
# define $name auto&
# define $readonly_name auto const&
#
# // Range-based `for`, because deduced `auto`can't be expressed via
e.g. `ref_`:
# define $each_value auto const
# define $each_ref auto&
# define $each auto const&
# define $in :
#
# define $repeat do{
# define $until(e) }while(not(e))
#
# define $when( condition ) condition?
# define $use( value ) value :
# define $default_to( value ) value
#
# define $_generate_using_declaration_in( ns, name ) using ns::name;
# define $using_from_namespace( ns, ... ) \
MM_APPLY_WITH_FIXED_ARG( $_generate_using_declaration_in, ns,
__VA_ARGS__ ) \
static_assert( true, "- just to support a semicolon after this -" )
#
# define $using_all_from_namespace( ns ) \
using namespace ns
#
# define $using_nested_namespace( ns, nested ) \
namespace nested = ns::nested
#
# define $lambda_using( ... ) [__VA_ARGS__]
# define $byref( object ) &object
# define $byval( object ) object
# define $capture_byref &
# define $capture_byval =
#
# define $lambda $lambda_using()
# define $lambda_using_references $lambda_using( $capture_byref )
# define $lambda_using_values $lambda_using( $capture_byval )
#
# define $hopefully( e ) $e::hopefully( e )
# define $fail( ... ) $e::fail_from_location( \
$as<std::string>( $funcname ), __VA_ARGS__ \
)
#
# // Can't use unqualified type builders here, hence the `$e` qualifiers.
# // The `(void)` casts avoids unused-warnings when these names are
unused.
# // Commonly `n_args` is named the more cryptic `argc` (with `c` for
`count`),
# // and commonly `args` is named the more cryptic `argv` (`v` for
`values`).
# // To use `n_args` and `args` you can pass a lambda as macro argument.
# // The macro is variadic to allow specification of a fatal error
handler.
# define $start_with( ... ) \
$function main( const int n_args,
$e::raw_array_<$e::ptr_<char>> args ) \
-> int \
{ \
(void) n_args; (void) args; \
return $e::default_startup( __VA_ARGS__ ); \
}
#
# // This is just an extra-convenience wrapper and doesn't support a fatal
# // error handler (modern C++ requires at least one argument for a
`...`).
# define $start_with_arguments( main_procedure ) \
$start_with( $lambda_using_values() \
{ \
main_procedure( {args, args + n_args} ); \
} )