ext/filter, Final API proposal

Hello,

Pierre wrote:

It always returns an array when FILTER_FLAG_ARRAY is given. Even when
the input value was a scalar. It spares yet another is_array/isset
test. And it accepts only scalar by default or when FILTER_FLAG_SCALAR
is given.

Makes sense but the example shows:

Again, this proposal is not based on the current neither on the
examples in the manual (I did not check them recently, there is maybe
mistakes).

a) This is a documentation bug
b) FILTER_FLAG_ARRAY is 'inherited' from the previous arg ('component')
c) The values are always returned as array

My favourite one being a) :-)

This is what this proposal says too, so I consider that you agree :)

It uses INPUT_REQUEST (which is not yet implemented, and will not be
for 5.2.0). One should rely only on the constants not their values :)

I agree wholeheartedly but I think it would be nice to have
INPUT_REQUEST for 5.2.0 already. Or are you advising people to use

Sadly no, it will not be in 5.2.0. We do not have the time, the
resources and the required tests to implement it before 5.2.0.

$myinputs = input_get_args(INPUT_GET, $args, $data) +
input_get_args(INPUT_POST, $args, $data) [ + ... ];

as work-around for now? Just wondering...

It will be array_merge(G,P,C,S) then, but this is not the goal of this
thread :-)

However, the point was about the API changes (signature, way to call
them, etc...). I suppose you like it?

Yes, the API looks great as far as I can tell.

Danke!

--Pierre

Hello,

unless FILTER_FLAG_ARRAY is given in which case it should behave like
before. This would make it easier to use the filtered values IMHO.

It always returns an array when FILTER_FLAG_ARRAY is given. Even when
the input value was a scalar. It spares yet another is_array/isset
test. And it accepts only scalar by default or when FILTER_FLAG_SCALAR
is given.

I would not expect that... I thought the FLAG would just mean that it
would iterate over a whole array and allow it. I think that we should
add a second flag called "FORCE_ARRAY" or something instead.

The flag allows arrays and always returns an array. It is obvious as
if you allow array or scalar, you have to use is_array, that's a step
we easily drop internally by always returning an array. I do not see
the point to have ALLOW_ARRAY and FORCE_ARRAY. It is confusing.

--Pierre

Agreed. input_id_filter() is shorter than input_name_to_filter(), and
input_list_filters() is the same length as input_filters_list().

Why not change the function prefixes to filter_*, which in turn
allows for MUCH shorter name,
Ex: input_list_filters -> filter_list()

Ilia Alshanetsky

Hello,

Minor: the general structure of the function names are
<prefix><verb><noun>, so it would be more consistent if this was
'input_list_filters'.

Overall: +1 (with bonus points if you go with <prefix><verb><noun>
for the two identified cases)

I for one think we should try to keep the names of something that
(hopefully) will be used often as short possible to reduce the likely
hood of typos.

Ilia Alshanetsky

Agreed. input_id_filter() is shorter than input_name_to_filter(), and
input_list_filters() is the same length as input_filters_list().

It should be something_to_something, not something_something. "is it
id_to_filter or id_filter?", no, I'm not going to ask me this question
in the next few years.

Also it is not a ID but its name, its ID is internal, numeric and
defined by a constant. name_to_filter reflects this fact, we can use
name_to_id if you prefer, but not id_to_filter.
However, that's nitpicking, you will use this function once a month
for debugging/testing/abstraction purposes.

input_filters_list or input_list_filters, I don't care, please choose one :)

--Pierre

Minor: the general structure of the function names are
<prefix><verb><noun>, so it would be more consistent if this was
'input_list_filters'.

Overall: +1 (with bonus points if you go with <prefix><verb><noun>
for the two identified cases)

I for one think we should try to keep the names of something that
(hopefully) will be used often as short possible to reduce the likely
hood of typos.

Ilia Alshanetsky

Agreed. input_id_filter() is shorter than input_name_to_filter(), and
input_list_filters() is the same length as input_filters_list().

This function won't be used a lot, so a longer name is fine. I would go
for input_get_filter_id() and input_list_filters().

regards,
Derick

Hello,

Before any further comment, we agreed on the "filter" prefix. We have
to stick to it. I also think we should respect other developers
choices.

As you said, filter is more than a simple extension, its design choice
should not be up to only one or two persons. My proposal has been
approved by many developers in this thread and in pecl-dev.

It seems that you either did not read the discussion or ignore the
popular choice. I do not think it is a good thing.

A patch with the changes we agreed on is already ready:

http://pecl.php.net/~pierre/filter_apishakeup.txt

It passes the tests (after update) and fix two bugs. That means a RC5
is possible this week, on friday.

But If you keep refuse to accept our choices, we first have to find
(again) compromises, get it approved, implement and test it. Given the
current tempo, php6 will be out before we are done! :P

However, my comments about your proposal (short version: no):

The major changes are:

• drop filter_data, input_get will use INPUT_DATA just like
  input_get_args

• input_get accepts takes no option or flag by default, only the filter
  type. If you need options or flags, you pass them and the filter type
  as array. Consistent and flexible.

I think that is a bad idea, and it's also something I disliked in the
_get_args() implementation as we'd basically be overloading the
parameter then. I would not want that for input_get(), and I think we
should also come up with something else for the definition in
input_get_args() there as well.

There is no other choices. Unless we add naming argument in php 5.2,
we are out of choicses. I also not think a function with five
arguments and one of them being mixed is a good thing, it is what we
did in the early versions of php, it was and is a mistake.

Well, you're still doing the same, but now with the parameter that also
contains the filter. This is fundamentally different compared to the
previous API, because both the flags and the options modify a filter,
and both of them are not required.

I know that both modify a filter. And I do think that array make the
code way more readable and easier to extend.

When I was first implementing this stuff I thought that it was
best to always use an array to specify both options and flags as 4th
parameter. However, there is only one filter (validate_int) that
currently uses the options (min_range and max_range); and I found that
having to use an array for all other filters just because of
validate_int was a bad idea, as it unnecessarily bloats calling
filters with funky array() syntax. Hence the status quo where you only
have to use an array if you want to use options besides flags at well.

More filters will use option (like IP for examples, or url, mails). An
array allows a clear and extendable signature. I don't think anyone
here can affirm that no other filter will use more options, flags or
arguments. The goal is to create an API that we will not have to break
in a near future, especially not because we added more filters. An
array gives us this security.

I think we should just stick to the old syntax here:

input_get(INPUT_GET, 'mystring', FILTER_SANITIZE_STRING,
FILTER_FLAG_ENCODE_HIGH|FILTER_FLAG_ENCODE_LOW );

And you have to use an array if you have options, add a fifth argument
for the upcoming charset, and a sixth if we need another one? That's
insane and confusing, sorry.

Which sixth parameter? And no, it's not more confusing then having to
use an array as third parameter only in case you want flags and or
options. As I said before: the filter is one part of the call, and the
filter's modifiers (options/flags) are. They don't logically belong
together in one parameter.

You are alone to find it more confusing. Actually you are alone to
want the old API.

In that case INPUT_DATA becomes ugly so I prefer a different function
for filtering already existing data in variables (as you're not
getting any input as "input_get" refers to).

It is consistent. All function works the same way. The same definition
works for both filter_get and filter_get_args.

Actually, it's less consistent and it would require another parameter
to the functions, while you just suggested that you disliked more
parameters. Again, it's also fundamentally two different things:

• request input data
• existing variables
  Therefore there should be two sets of functions, one of the classes with
  both a single filter specification and one with a filter specification
  array.

It makes no difference to filter a variable or input data. Having both
modes in one function allows easy integration of the filter API into a
more generic API. We also agreed here on this choice.

Droping custom functions to filter variables (from script, not input
data) has been requested and approved. I think it is a good thing.

Sorry, but I dislike this one, it does not the need of consistency and
clarity. I also thought about a similar API (the same without the
variable functions), but using an array for all arguments (as proposed
in my final version) is the way to go. Using the 3rd argument as flag
or array just brings us back to the current API, which is confusing.

input_has_variable( $source, $varName );
input_get_filter_id( $filterName )
input_filters_list();

These three functions names have been approved and I prefer what we choose.

From the mail about how FLAG_ARRAY should work:

I would not expect that... I thought the FLAG would just mean that
it would iterate over a whole array and allow it. I think that we
should add a second flag called "FORCE_ARRAY" or something instead.

The flag allows arrays and always returns an array. It is obvious

No it isn't obvious at all. Look at this example:

<?php
$varSpecs = array(
'varName' => array( 'filter' => 'string', 'flags' => FILTER_FLAG_ARRAY )
);

$data = array( 'varName' => 42 );

$filteredData = input_filter_args( $data, $varSpecs );
?>

Because the variable 'varName' has the FILTER_FLAG_ARRAY you say that it
should make an array out of this, resulting in the data:

$filterData = array( 'varName' => array( 42 ) );

Unless you want to ignore this flag for filtering already existing
variables which makes things inconsistent again.

I do not understand what you mean here.

Let me explain it again, if the flag is not set and the input is an
array, it returns false (validation fails). If it is present, you are
sure to get an array in return, even if the variable was a scalar.

Therefore we should use use the FILTER_FLAG_ARRAY to recursively go over
the input variable that is being filtered. If the flag is not there and
the input variable is an array, it should return false/null (whatever
the default is).

That's what it does when FILTER_FLAG_ARRAY is set. FILTER_FLAG_SCALAR
being the default.

--Pierre