Re: [Clops-users] new signature of the parse method

[Mikoláš J. <mik...@gm...>]

couple of remarks:

- for some users it might be enough to just say the usage and drop the
error messages (e.g. 'cp foo' just does that).

- it seems to me there's some additional burden on the user to know
that the parsing was ok iff the list of errors is empty

- where does the report method come from? providing and external
printing might could be useful if we wanted to provide several
standard ways of printing errors but the user has to find those
somewhere (some additional doc reading required)

all in all, do we go for the wrapper then?

m.


On Sun, Jul 26, 2009 at 9:53 PM, Radu Grigore<rad...@gm...> wrote:
> On Sun, Jul 26, 2009 at 3:54 PM, Fintan Fairmichael<fi...@gm...> wrote:
>> We discussed this on Friday and had already agreed such.
>
> That said, both reasons given by Mikolas are invalid.
>
>> On Sun, Jul 26, 2009 at 1:23 PM, Mikoláš Janota <mik...@gm...>
>> wrote:
>>>  a) To get the previous code to compile I added isEmpty() which looks
>>> unintuitive. isOK would look nicer.
>
> If the old version was
>  if (!p.parse(args)) { printUsage(); return; }
> you can't simply transform it to
>  if (!p.parse(args).isEmpty()) { ... }
> because that would loose the error messages that where previously
> reported as exceptions. If you want to not loose functionality you
> must say:
>  List<CLError> errors = p.parse(args);
>  if (!errors.isEmpty()) {
>    report(errors, System.err); // well... if report() is added to CLError
>    printUsage();
>    return;
>  }
> In fact, this is slightly better than the original because it prints
> the usage information whenever the user had an error on the command
> line, not only for those errors that just happened not to throw. In
> this form, it seems quite natural to test whether there are any errors
> by saying "!errors.isEmpty()" and isOK() would look strange.
>
> In short, the examples are incorrect now. But they should be fixed
> after these API changes are stabilized anyway.
>
>>>  b) The standard toString of List is probably not desirable for
>>> presenting to the user the aforementioned class could have some
>>> printMe method, presenting the errors nicely.
>
> With a list you say
>  report(errors, System.err);
> and with a wrapping class you say
>  parsingResult.reportErrors(System.err);
> which is not much different.
>
> The toString() method is for debugging (although there are some *very*
> rare situations when I'd use it in a different way). I don't think
> anyone ever considered it for reporting the parsing errors.
>
> So, why would we use a wrapping class? The best reason I see is that
> it gives us a layer of abstraction that we could exploit later on if
> we have to make changes. In other words, it gives us more freedom to
> change the implementation while keeping the interface unchanged. For
> example, if for some reason we notice that errors are better organized
> in a tree (say, A is a child of B if A seems to be causing B) then we
> can just _add_ functions to the interface that expose the tree
> structure, keep reportErrors(), and *nothing* would need to change in
> the user code.
>



-- 
Mikoláš Janota M. Sc.
School of Computer Science and Informatics,
University College Dublin,
Belfield,
Dublin 4,
Ireland