Drop down List type ahead auto completion suggestions

[Swetha]

Notes:
 when people search for something in the search bar then they should
be provided with a drop down list so that it will be easy for them to
select the links instead of them typing the whole word.

For Example:
 Suppose you are searching for "food stamps" and you start tying the
word say "foo" then you should see a drop down list which contains
options like food,food centers,food organizations,food stamps etc.,so
that the user instead of typing the whole word the user can select
the choice from the drop down list.

[Dan MacNeil (omacneil)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

  status confirmed
  done

Other words for this feature are "type ahead" and "auto completion"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFKbdE4LzI3mETyffwRAqdVAJ9kFGyvJWzNZGMa2Nmcevw3E7p01gCgkXOF
CjI5pmxAbijvwFoNqyCtcvg=
=tE17
-----END PGP SIGNATURE-----

[Dan MacNeil (omacneil)]

Code was much clearer than I am used to getting.

# Generally in a library private methods/subs should be prefixed with a:

# '_'

# generally, barring life and death the only way
# into a sub is through parameters and the only
# way out is through return values

# assert doesn't seem to be called anywhere
# also we favor calling
# MVHub::Utils::assert over importing it and
# calling as 'assert'

     4 use MVHub::Utils 'assert';

# should be $suggestions_aref
# also no reason not to declare it inside build_suggestion_list
# globals are by default naughty

    11 our $suggestions = [];

# Should be defined via MVHub::Utils::ConfigSimple in conf file
# maybe $MAX_LIST_SIZE_SEARCH_SUGGESTIONS
# should also be accessed inside push_suggestions

    12 # set this to the max number of items you want to display in a
set
    13 our $limit = 10;

# globals naughty, if global is needed maybe better as closure
# see: http://etutorials.org/Programming/Perl+objects,+references+and
+modules/Chapter+6.+Subroutine+References/6.7+Closure+Variables+as
+Static+Local+Variables/

    14 our $item_count = 0;

# in persistant enviroment, where suggest.pl never exists with program
running all time
# this will be problimatic.
    14 our $item_count = 0;

# consider removing this sub
# putting the push inline in build_suggestion_list
# avoids globals, uses less code and maybe is
# clearer.
    18 sub push_suggestion {

# should probably declare and return $suggestions_aref
# over the global
    28 sub build_suggestion_list {

# just 'return' will do right thing
# in scalar or list context
# also a bit confusing since this sub
# is currently used only in void context

    32 return [];

# with only one $sql, used immeadiately variable name doesn't matter
# here $sql_something_descriptive is better
    36 my $sql1 =
MVHub::Utils::DB::get_sql_select_statement('AGENCY_X_SEARCH_WITH_AUTOCOMPLETE');
    37 my $sql2 =
MVHub::Utils::DB::get_sql_select_statement('ALL_NAMES_VIEW_CATS_WITH_VISIBLE_PROGRAMS_VIEW_X_ALL_FOR_SYNONYMS_WITH_AUTOCOMPLETE');

# if there are 10 agencies, no categories will
# be returned, this is probably fine, but should
# be deliberate
    50 foreach my $agency (@$agencies_aref) {
    54 foreach my $category (@$categories_aref) {

# CGI::Application is responsible for output
# also don't want to output until last
# minute so as not to break test code and
# endure hassle of side effects.
    61 print "Content-type: application/json\n\n[";
    67 print "\"$_\"";
    70 print "]\n";

# better
# $suggestions_aref =build_suggestion_list(...)
    78 build_suggestion_list( $dbh, $search_phrase );

[Dan MacNeil (omacneil)]

MINOR NIT EXAMPLE
There are some **very** minor nits, that I'll detail in a separate
message, for example:

 my $foo=qq{"string"};

is more readable than:

        my $foo="\"string\""
see:
     man perlop

INTERFACE NITS
Having developers argue about what the users want is infinately inferior
to testing what they want but...

1) It would be good if the suggestions adjusted as the user types more
letters. Typing "U" then T" should be different than "U"

2) "%$word% confused me. I had too look to understand that I was
getting

3) The category search was tough to understand, probably a net negative.

***draft*** NEXT STEPS / Roadmap

Someplace we need test code....

Someplace we should study existing literature on search implementations

1) add program word search that works like existing agency word search,
and puts results in existing search output format.

1-a) commit code / push to production

2) remove category search from jquery stuff

3) Add program word search to jquerry

3a) limit jquery to suggestions that match at start of string.

4) do usability testing.

 ---I can raise a few hundred bucks for this.

        ----We can improve searching for agencies / programs

######## Semester ends ??? ############

5) revamp search to be faceted / rank results.

[Dan MacNeil (omacneil)]

Re globals vs member variables.

The belabor the point, (beat the horse to glue?) Accessing globals directly, not through parameters is naughty because:

 1) It's hard to see what's going on.
 2) It becomes difficult to pull the function out and use it outiside the file it was defined and outside the scope of the global-like thing
         ---Like in a test file
 3) it encourages side effects at a distance.
     ---you don't know if your changes to the var will effect something far away oand
4) if the implementation changes you are screwed.

This is (especially #4) is also true of member variables of a class, in either C++ or perl.

give a var $limit, better to say $self->{limit}; $limit and better still to say:

$self->get('limit');

[Dan MacNeil (omacneil)]

See also:
  man HTML::Template
  man CGI::Applicaiton
  perldoc MVHub::CGIAppBase

Should remove these lines before a merge request, commented out code is a warning sign. Version control software lets us revert should we change our mind
    9 # set this to the max number of items you want to display in a set
    10 #our $limit = 10;

Still a side effect, not something I'd deny a merge for but not ideal.
    22 push @$suggestions_aref, lc $suggestion;

Could fit line 22 in between 57 & 58
    57 foreach my $agency (@$agencies_aref) {
    58 my $name = $$agency{agency_name};

# untested suggestion
foreach my $suggestion_href (@$agencies_aref,@$categories_aref){
# requires changing SQL
     push @$suggestions_aref,$$suggestion_href{name}
    last if ++$item_count > $MAX_ITEMS;
}

Still Bad: how is concat operator harder than print ?
    73 print "Content-type: application/json\n\n[";
    77 print ',';
    79 print "\"$_\"";
    82 print "]\n";

If a sub isn't used outside this file it gets a leading _
  27 sub build_suggestion_list {
   71 sub output_json_array {

[Dan MacNeil (omacneil)]

#optional SQL change
SELECT
    agency_name as name
....
SELECT
   category_name as name

[Dan MacNeil (omacneil)]

After merging trunk in.

this test:

        app-mvhub/t/conf_files_match.t

fails.

Probably because mv_update_development is buggy and doesn't correctly copy value from template.conf

also:
    app-mvhub/t/perltidy.t

should be obvious

app-mvhub/t/perltidy.t fails because it hasn't been updated for your new queries.

[csmith (csmith-thecsl)]

I made most of the changes you've outlined so far, the biggest being
removing push_suggestion and making the suggest module conform to the
CGI::Application standards.

I don't quite know how to better the search interface or make the category
search less confusing... I am open to suggestions of course. I think
removing push_suggestion made the code a lot more readable at any rate.

Let me know what you think,
Chris

> MINOR NIT EXAMPLE
> There are some **very** minor nits, that I'll detail in a separate
> message, for example:
>
> my $foo=qq{"string"};
>
> is more readable than:
>
> my $foo="\"string\""
> see:
> man perlop
>
> INTERFACE NITS
> Having developers argue about what the users want is infinately inferior
> to testing what they want but...
>
> 1) It would be good if the suggestions adjusted as the user types more
> letters. Typing "U" then T" should be different than "U"
>
> 2) "%$word% confused me. I had too look to understand that I was
> getting
>
> 3) The category search was tough to understand, probably a net negative.
>
>
> ***draft*** NEXT STEPS / Roadmap
>
> Someplace we need test code....
>
> Someplace we should study existing literature on search implementations
>
> 1) add program word search that works like existing agency word search,
> and puts results in existing search output format.
>
> 1-a) commit code / push to production
>
> 2) remove category search from jquery stuff
>
> 3) Add program word search to jquerry
>
> 3a) limit jquery to suggestions that match at start of string.
>
> 4) do usability testing.
>
> ---I can raise a few hundred bucks for this.
>
> ----We can improve searching for agencies / programs
>
> ######## Semester ends ??? ############
>
> 5) revamp search to be faceted / rank results.
>
> --
> Drop down List type ahead auto completion suggestions
> https://bugs.launchpad.net/bugs/403596
> You received this bug notification because you are a bug assignee.
>
> Status in MVHub, a directory of social services: Confirmed
>
> Bug description:
> Notes:
>  when people search for something in the search bar then they should
> be provided with a drop down list so that it will be easy for them to
> select the links instead of them typing the whole word.
>
> For Example:
>  Suppose you are searching for "food stamps" and you start tying the
> word say "foo" then you should see a drop down list which contains
> options like food,food centers,food organizations,food stamps etc.,so
> that the user instead of typing the whole word the user can select
> the choice from the drop down list.
>