www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.xml validity checking is absurd

reply Stewart Gordon <smjg_1998 yahoo.com> writes:
Apologies if this has been talked about before.  I haven't been able to find it
by a quick 
search of the 'group.  Apologies also if what I'm saying is already taken care
of in the 
module that's being drafted as a replacement for std.xml.

This is what I've found: Validity checking is done in an in contract!

This is saying it's _illegal_ to construct a Document or DocumentParser from
invalid XML, 
not just that it's an error condition.  This is contrary to the spirit of DBC
and 
exception handling.  DBC is for detecting program bugs.  Exception handling is
for dealing 
with unexpected conditions at runtime.

99% of the time, XML data will come from a file or an external process, rather
than being 
hard-coded in the program or even generated by and passed from another part of
the 
program.  As such, invalid XML input is an unexpected condition at runtime,
verifying that 
the XML is syntactically and structurally valid is part of what the program
needs to do. 
An invalid XML file is not a sign of a bug in a program - indeed, the failure
to detect 
the invalidity of an XML file is.

This means that rather than just

Document data = new Document(xml);

you need to do

check(xml);
Document data = new Document(xml);

Consequently, in a development build the XML is parsed three times:
- first, through the call to the check function here
- then, when check is called again in DocumentParser's constructor's in contract
- and finally, in the body of the Document constructor as it is actually
building the DOM.

This shouldn't be necessary.  Validity should be checked automatically while
parsing the 
XML to build the DOM.  This would mean that the XML is parsed only once, which
is much 
more efficient as well as being a first step towards enabling the XML to be
read from a 
stream and parsed on the fly.

And it should throw a normal exception if it fails, not an assertion failure. 
I haven't 
taken the time to figure out what actually does happen if malformed XML is
passed in in a 
release build.  But I don't suppose that it errors out gracefully in the
general case.

Anyway ... is there going to be a fix for this, or is it just a case of waiting
for the 
replacement for std.xml and using the workaround or an alternative XML parser
in the meantime?

Stewart.
Feb 07 2013
next sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Thursday, 7 February 2013 at 22:22:09 UTC, Stewart Gordon 
wrote:

 This is what I've found: Validity checking is done in an in 
 contract!


I've ran into the same problem with std.base64. DbC doesn't seem 
to be a generally well-understood concept.
Feb 07 2013
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/7/13 5:27 PM, Vladimir Panteleev wrote:

 On Thursday, 7 February 2013 at 22:22:09 UTC, Stewart Gordon wrote:

 This is what I've found: Validity checking is done in an in contract!


 I've ran into the same problem with std.base64. DbC doesn't seem to be a
 generally well-understood concept.


That's why TDPL dedicates a whole chapter to it (separate from error 
handling!). Apparently that didn't make a dent in the Universe :o).

Andrei
Feb 07 2013
next sibling parent Marco Leise <Marco.Leise gmx.de> writes:
Am Thu, 07 Feb 2013 17:36:53 -0500
schrieb Andrei Alexandrescu <SeeWebsiteForEmail erdani.org>:


 On 2/7/13 5:27 PM, Vladimir Panteleev wrote:

 On Thursday, 7 February 2013 at 22:22:09 UTC, Stewart Gordon wrote:

 This is what I've found: Validity checking is done in an in contract!


 I've ran into the same problem with std.base64. DbC doesn't seem to be a
 generally well-understood concept.


 
 That's why TDPL dedicates a whole chapter to it (separate from error 
 handling!). Apparently that didn't make a dent in the Universe :o).
 
 Andrei


I'm just thinking that http://wiki.dlang.org/Phobos could be
extended by a review process check-list which includes
"correct use of DBC", "use of ranges where appropriate" and
whatever else is desired for Phobos. Another term would be
Quality Standards.

-- 
Marco
Feb 07 2013
prev sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 7 February 2013 at 22:36:53 UTC, Andrei Alexandrescu 
wrote:

 On 2/7/13 5:27 PM, Vladimir Panteleev wrote:

 On Thursday, 7 February 2013 at 22:22:09 UTC, Stewart Gordon 
 wrote:

 This is what I've found: Validity checking is done in an in 
 contract!


 I've ran into the same problem with std.base64. DbC doesn't 
 seem to be a
 generally well-understood concept.


 That's why TDPL dedicates a whole chapter to it (separate from 
 error handling!). Apparently that didn't make a dent in the 
 Universe :o).

 Andrei


"in" and "out" contracts themselves are flawed in D in any case, 
given they are part of the "called" code, as opposed to "caller" 
code.

This makes them absolutely no different than an assert.

The problem is that an assert is "internal" validation, whereas 
an "in"/"out" is supposed to be a handshake between the 
caller/callee.

If I write an "sqrt" function, and document it as "Please, only 
give me positive numbers", and then write a contract for it, and 
then compile my lib in release, the caller will have no way of 
"signing" my contract.

He'll call my sqrt with negative numbers, and the in will never 
get called, and sqrt will crash horribly.

A *BLATANT* example of this limitation is slice operations: They 
have an in contract stating that the slices need to be the same 
length. However, this contract will never ever get run, for 
anyone, because druntime is built and distributed in release. 
Long story short, even if I compile in debug, the code will 
silently run erroneously.

http://d.puremagic.com/issues/show_bug.cgi?id=8650

Please see also:
http://d.puremagic.com/issues/show_bug.cgi?id=4720
http://d.puremagic.com/issues/show_bug.cgi?id=6549

And finally, this old thread about the subject, which kind of 
fell into darkness:
http://forum.dlang.org/thread/jamrtmgozgtswdadeocg forum.dlang.org
Feb 08 2013
next sibling parent Dmitry Olshansky <dmitry.olsh gmail.com> writes:
08-Feb-2013 15:25, monarch_dodra пишет:

 On Thursday, 7 February 2013 at 22:36:53 UTC, Andrei Alexandrescu wrote:

 On 2/7/13 5:27 PM, Vladimir Panteleev wrote:

 On Thursday, 7 February 2013 at 22:22:09 UTC, Stewart Gordon wrote:

 This is what I've found: Validity checking is done in an in contract!


 I've ran into the same problem with std.base64. DbC doesn't seem to be a
 generally well-understood concept.


 That's why TDPL dedicates a whole chapter to it (separate from error
 handling!). Apparently that didn't make a dent in the Universe :o).

 Andrei


 "in" and "out" contracts themselves are flawed in D in any case, given
 they are part of the "called" code, as opposed to "caller" code.


+111


 This makes them absolutely no different than an assert.

 The problem is that an assert is "internal" validation, whereas an
 "in"/"out" is supposed to be a handshake between the caller/callee.

 If I write an "sqrt" function, and document it as "Please, only give me
 positive numbers", and then write a contract for it, and then compile my
 lib in release, the caller will have no way of "signing" my contract.

 He'll call my sqrt with negative numbers, and the in will never get
 called, and sqrt will crash horribly.



-- 
Dmitry Olshansky
Feb 08 2013
prev sibling next sibling parent "Dicebot" <m.strashun gmail.com> writes:
On Friday, 8 February 2013 at 11:25:07 UTC, monarch_dodra wrote:

 "in" and "out" contracts themselves are flawed in D in any 
 case, given they are part of the "called" code, as opposed to 
 "caller" code.

 This makes them absolutely no different than an assert.

 The problem is that an assert is "internal" validation, whereas 
 an "in"/"out" is supposed to be a handshake between the 
 caller/callee.


Isn't it exactly how contracts are supposed to work? They are 
neither error handling tools, nor handshake - more like tools to 
verify interconnections between different inner parts of program 
stay sane. For example, in web application you would like to 
escape user input only once, upon receiving it, and it makes 
sense to not waste cycles on verifying it is actually escaped 
down the code path. But adding a contract to make sure someone 
does not use that library function on a raw input by accident - 
makes perfect sense. Of course, contracts shine only if some 
debug-mode testing is in place during development process but 
right now they do behave in D exactly how I would have supposed.
Feb 08 2013
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/8/13 6:25 AM, monarch_dodra wrote:

 "in" and "out" contracts themselves are flawed in D in any case, given
 they are part of the "called" code, as opposed to "caller" code.


What would be the right design and implementation?


 This makes them absolutely no different than an assert.

 The problem is that an assert is "internal" validation, whereas an
 "in"/"out" is supposed to be a handshake between the caller/callee.

 If I write an "sqrt" function, and document it as "Please, only give me
 positive numbers", and then write a contract for it, and then compile my
 lib in release, the caller will have no way of "signing" my contract.


I don't think std.math.sqrt should validate its input using a contract.


 He'll call my sqrt with negative numbers, and the in will never get
 called, and sqrt will crash horribly.


It'll return NaN.


 A *BLATANT* example of this limitation is slice operations: They have an
 in contract stating that the slices need to be the same length. However,
 this contract will never ever get run, for anyone, because druntime is
 built and distributed in release. Long story short, even if I compile in
 debug, the code will silently run erroneously.

 http://d.puremagic.com/issues/show_bug.cgi?id=8650


That druntime uses a contract to verify length in slice is an 
antipattern. It should use a sheer test and throw.


 Please see also:
 http://d.puremagic.com/issues/show_bug.cgi?id=4720


This is legit. We should have a way to separate contracts from body in 
the general case.


 http://d.puremagic.com/issues/show_bug.cgi?id=6549


This, too, is legit.


 And finally, this old thread about the subject, which kind of fell into
 darkness:
 http://forum.dlang.org/thread/jamrtmgozgtswdadeocg forum.dlang.org


Yeah, makes sense. I don't think we should put contracts on the front 
burner. For whatever reason, people don't use contracts or they misuse 
them. I have no idea why. The obvious argument is that people would use 
contracts if this or that bug were fixed, but there's past evidence 
suggesting the contrary.


Andrei
Feb 08 2013
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Andrei Alexandrescu:


 I don't think std.math.sqrt should validate its input using a 
 contract.


Why not? Returning a NaN is an option. But in a language that 
supports contracts, its precondition should only accept 
non-negative inputs.



 That druntime uses a contract to verify length in slice is an 
 antipattern.


Why?



 I don't think we should put contracts on the front burner.


Why? (And I don't agree).



That's why TDPL dedicates a whole chapter to it (separate from 
error


handling!). Apparently that didn't make a dent in the Universe 
:o).<

Instead of not putting contracts on the front burner, I suggest 
to think about the causes, find some of them, and then try to fix 
the causes.



 For whatever reason, people don't use contracts or they misuse 
 them. I have no idea why. The obvious argument is that people 
 would use contracts if this or that bug were fixed, but there's 
 past evidence suggesting the contrary.


I am using D contracts in my code. I think there are three or 
more reasons for people not using D contracts a lot:
- C/C++/Python/Java programmers don't know much about contract 
programming. They look like improved asserts, while they require 
a bit different mindset. Learning a different mindset requires 
time and work. This is an important point but I think this isn't 
the most important one.
- D contracts miss some important parts, the pre-state is one of 
them. Other problems are present (as they say "move in contract 
checking to the caller side").
- I think Phobos contracts get removed. This reduces their 
usefulness. If you have a feature X in your language and you 
aren't using it in your standard library, then a bit of redesign 
is needed.
- D is used a system language, where often programmers think of 
asserts as things present only in debug builds. But maybe 
contracts should have a different compiler switch to be stripped 
away (http://d.puremagic.com/issues/show_bug.cgi?id=9482 ).

I also think it's good to reduce the usage of enforce() in 
contracts, because I think it doesn't help the cause of D 
Contracts. I think well implemented Contracts don't need 
exceptions.

Bye,
bearophile
Feb 08 2013
next sibling parent reply "bearophile" <bearophileHUGS lycos.com> writes:
Stewart Gordon has opened this ER (worth voting for):

http://d.puremagic.com/issues/show_bug.cgi?id=9483

Bye,
bearophile
Feb 08 2013
parent FG <home fgda.pl> writes:
On 2013-02-08 14:59, bearophile wrote:

 Stewart Gordon has opened this ER (worth voting for):
 http://d.puremagic.com/issues/show_bug.cgi?id=9483


A good proposal now, when it's the callee that does contract validation.
And easier to do than "move in contract checking to the caller side".
Feb 08 2013
prev sibling parent "John Colvin" <john.loughran.colvin gmail.com> writes:
On Friday, 8 February 2013 at 13:33:43 UTC, bearophile wrote:

 Andrei Alexandrescu:


 I don't think std.math.sqrt should validate its input using a 
 contract.


 Why not? Returning a NaN is an option. But in a language that 
 supports contracts, its precondition should only accept 
 non-negative inputs.

 Bye,
 bearophile


Passing a negative number to sqrt is something that could well be 
an expected event in release code that works with real data.

A Nan is a convenient way of marking bad data (and anything 
calculated from that data) as being invalid without having to set 
up an exception framework that does the same job explicitly and 
inefficiently.

If sqrt used contracts, then that would make debug mode useless 
to a lot of data analysis programs that are designed (sensibly) 
to use NaNs.
Feb 08 2013
prev sibling next sibling parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 02/08/2013 04:54 AM, Andrei Alexandrescu wrote:


 If I write an "sqrt" function, and document it as "Please, only give me
 positive numbers", and then write a contract for it, and then compile my
 lib in release, the caller will have no way of "signing" my contract.


 I don't think std.math.sqrt should validate its input using a contract.


As the module writer, I don't know whether my function is going to be 
used as an "API function" or as an "internal function" in somebody 
else's module.

In the former case, I would like my checks to be enforce() calls so that 
I don't produce incorrect results.

However, in the latter case, I would like my checks to be in an 'in' 
block so that when my client compiles in release mode, my checks would 
disappear. This is under the assumption that my client has tested their 
code and they are sure that all of the contracts are met. There is no 
need for them to waste time running the checks unnecessarily.

Ali
Feb 08 2013
parent reply "Dicebot" <m.strashun gmail.com> writes:
On Friday, 8 February 2013 at 16:11:14 UTC, Ali Çehreli wrote:

 As the module writer, I don't know whether my function is going 
 to be used as an "API function" or as an "internal function" in 
 somebody else's module.


Then it is an API function, as far as I understand DbC, and 
should never rely on contract to validate input.
Feb 08 2013
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 02/08/2013 08:18 AM, Dicebot wrote:

 On Friday, 8 February 2013 at 16:11:14 UTC, Ali Çehreli wrote:

 As the module writer, I don't know whether my function is going to be
 used as an "API function" or as an "internal function" in somebody
 else's module.


 Then it is an API function, as far as I understand DbC, and should never
 rely on contract to validate input.


It is an internal function if I use it so:

import std.math;
import std.exception;

void foo(double a, double b)
{
     enforce(a > b, "a must be greater than b");
     sqrt(a - b);
}

void main()
{}

See how sqrt() is *not* on my module's API? I have already enforced that 
my API function foo() is called correctly. My call to sqrt() is an 
internal call of my module. Therefore the checks inside sqrt() should 
not be repeated beyond my testing stage.

Ali
Feb 08 2013
next sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Friday, 8 February 2013 at 17:16:25 UTC, Ali Çehreli wrote:

 On 02/08/2013 08:18 AM, Dicebot wrote:

 On Friday, 8 February 2013 at 16:11:14 UTC, Ali Çehreli wrote:

 As the module writer, I don't know whether my function is




 going to be

 used as an "API function" or as an "internal function" in




 somebody

 else's module.


 Then it is an API function, as far as I understand DbC, and


 should never

 rely on contract to validate input.


 It is an internal function if I use it so:

 import std.math;
 import std.exception;

 void foo(double a, double b)
 {
     enforce(a > b, "a must be greater than b");
     sqrt(a - b);
 }

 void main()
 {}

 See how sqrt() is *not* on my module's API? I have already 
 enforced that my API function foo() is called correctly. My 
 call to sqrt() is an internal call of my module. Therefore the 
 checks inside sqrt() should not be repeated beyond my testing 
 stage.

 Ali


What is the point of an "in" contract, if not to check that the 
*caller* code is correct? Still assuming "sqrt" can't handle 
negatives, why bother with an "in", when you could just as well 
assert?

Heck, at this point, how is an "in" any different than a 
"version(assert) {...}", and an "out" an "version(assert) 
scope(exit) { ... }" ?

compile this program in debug:
//----
void main()
{
   auto a = [1, 2, 3];
   auto b = [1, 2];
   a[] += b[];
}
//----
Well herp dee derp! Look at it silently running and failing! The 
idea behind a debug build is that I should get an error for this 
kind of crap. But I don't. That's bad.

No end-user should be using a non-release version of druntime: 
That has  already been validated. On the other hand, once 
compiled in release, all the checks that *should* be executed 
when the caller is trying to validate his code *aren't* run.

Regardless of "where" the contract is compiled in (implementation 
detail), its inclusion (or lack thereof) should be at the 
callers' discretion.
Feb 08 2013
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 02/08/2013 09:55 AM, monarch_dodra wrote:


 Regardless of "where" the contract is compiled in (implementation
 detail), its inclusion (or lack thereof) should be at the callers'
 discretion.


In case it is not obvious, I agree with all you said. :)

Ali
Feb 08 2013
parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Friday, 8 February 2013 at 18:06:34 UTC, Ali Çehreli wrote:

 On 02/08/2013 09:55 AM, monarch_dodra wrote:


 Regardless of "where" the contract is compiled in


 (implementation

 detail), its inclusion (or lack thereof) should be at the


 callers'

 discretion.


 In case it is not obvious, I agree with all you said. :)

 Ali


Stupid text-based communication.

Sorry.
Feb 08 2013
prev sibling parent FG <home fgda.pl> writes:
On 2013-02-08 18:16, Ali Çehreli wrote:

 void foo(double a, double b)
 {
      enforce(a > b, "a must be greater than b");
      sqrt(a - b);
 }

 void main()
 {}

 See how sqrt() is *not* on my module's API? I have already enforced that my API
 function foo() is called correctly. My call to sqrt() is an internal call of my
 module. Therefore the checks inside sqrt() should not be repeated beyond my
 testing stage.



This calls for some way to be able to call sqrt without running checks:

     sqrt(a - b);   // would run all "in" checks
     #sqrt(a - b);  // tells the compiler to bypass the "in" checks in sqrt

Sorry for inventing clumsy syntax, but you get the picture.
Naturally it won't bypass checking if sqrt is in a compiled library.
Feb 08 2013
prev sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Friday, February 08, 2013 07:54:52 Andrei Alexandrescu wrote:

 On 2/8/13 6:25 AM, monarch_dodra wrote:

 "in" and "out" contracts themselves are flawed in D in any case, given
 they are part of the "called" code, as opposed to "caller" code.


 
 What would be the right design and implementation?


The way that it _should_ work but doesn't (and probably can't given D's 
linking model) is to insert in and out contracts at the call site so that it's 
the caller's compilation flags which decide whether the contracts are compiled 
in or not. They're testing the _caller's_ code after all. But the way that 
it's currently done, it's up to whoever released the library to decide whether 
the contracts are compiled in or not. For instance, it would ideally be 
possible for a program to be built with all of the debug stuff turned on 
(including assertions) and link against Phobos and have all of the in and out 
contracts on Phobos functions enabled (because the caller has assertions 
enabled). Instead, you have to build Phobos with assertions turned on in order 
to get them, even though you don't care about assertions which are internal to 
Phobos and don't need any debugging anything enabled in Phobos beyond the in 
and out contracts (since you're not debugging Phobos, just your code).

Unfortunately, while that's how it really _should_ work, AFAIK, there's no way 
with D's linking model to make things work that way. You can link against 
functions without any access to their bodies. Function pointers make it 
trivial to use a function without the compiler knowing what function your 
using (meaning that it couldn't insert the contracts at the call point). Etc. 
Etc. The contracts would have to be passed around with the functions in a 
manner which made it so that the caller could always insert them if it's being 
compiled with assertions enabled, and that just won't work.

So, yes. The implementation of D's contracts is flawed in the sense that it 
doesn't work the way that it would ideally work, but I don't think that it's 
actually possible to make it work the way that it would ideally work.

- Jonathan M Davis
Feb 08 2013
prev sibling parent Stewart Gordon <smjg_1998 yahoo.com> writes:
On 08/02/2013 11:25, monarch_dodra wrote:
<snip>

 "in" and "out" contracts themselves are flawed in D in any case, given they
are part of
 the "called" code, as opposed to "caller" code.


<snip>

I think you'll find that this is an implementation issue, rather than a
language issue.

It's been discussed many times over, but it seems that for some reason it's
still too 
controversial to get a fix any time soon.  Meanwhile, I've filed these to try
and soften 
the blow:

http://d.puremagic.com/issues/show_bug.cgi?id=9482
http://d.puremagic.com/issues/show_bug.cgi?id=9483

Stewart.
Feb 08 2013
prev sibling next sibling parent reply Nick Sabalausky <SeeWebsiteToContactMe semitwist.com> writes:
std.xml itself is known to be absurd. I haven't used it myself,
but there's been a general consensus for years that it needs a complete
rewrite. Attempts have been started, but nobody's finished one,
AFAIK. You could try the XML module in Tango, it's supposed to be
ridiculously fast.
Feb 07 2013
next sibling parent David <d dav1d.de> writes:
Am 07.02.2013 23:35, schrieb Nick Sabalausky:

 std.xml itself is known to be absurd. I haven't used it myself,
 but there's been a general consensus for years that it needs a complete
 rewrite. Attempts have been started, but nobody's finished one,
 AFAIK. You could try the XML module in Tango, it's supposed to be
 ridiculously fast.
 


There is also http://opticron.no-ip.org/svn/branches/kxml/
Feb 07 2013
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On 2013-02-07 23:35, Nick Sabalausky wrote:

 std.xml itself is known to be absurd. I haven't used it myself,
 but there's been a general consensus for years that it needs a complete
 rewrite. Attempts have been started, but nobody's finished one,
 AFAIK. You could try the XML module in Tango, it's supposed to be
 ridiculously fast.


It is fast, but it doesn't perform any validations. I have no idea what 
happens if pass invalid XML to it.

-- 
/Jacob Carlborg
Feb 08 2013
parent "deadalnix" <deadalnix gmail.com> writes:
On Friday, 8 February 2013 at 11:06:56 UTC, Jacob Carlborg wrote:

 On 2013-02-07 23:35, Nick Sabalausky wrote:

 std.xml itself is known to be absurd. I haven't used it myself,
 but there's been a general consensus for years that it needs a 
 complete
 rewrite. Attempts have been started, but nobody's finished one,
 AFAIK. You could try the XML module in Tango, it's supposed to 
 be
 ridiculously fast.


 It is fast, but it doesn't perform any validations. I have no 
 idea what happens if pass invalid XML to it.


The fact is that it isn't fast at the end. Tango.xml beat it 
pretty badly.
Feb 08 2013
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/7/13 5:22 PM, Stewart Gordon wrote:

 Apologies if this has been talked about before. I haven't been able to
 find it by a quick search of the 'group. Apologies also if what I'm
 saying is already taken care of in the module that's being drafted as a
 replacement for std.xml.

 This is what I've found: Validity checking is done in an in contract!


Please submit a bug report. Thanks!

Andrei
Feb 07 2013
parent Stewart Gordon <smjg_1998 yahoo.com> writes:
On 07/02/2013 22:36, Andrei Alexandrescu wrote:
<snip>

 Please submit a bug report. Thanks!


http://d.puremagic.com/issues/show_bug.cgi?id=9471
Feb 07 2013
prev sibling parent reply Stewart Gordon <smjg_1998 yahoo.com> writes:
Moreover, what do people think about the situation whereby check's sole payload
is to 
possibly throw an exception?

Normally under an exception-handling system, the behaviour of a function is: Do
something, 
or if you cannot, then throw an exception.

But what is the "something" in the case of check?  As far as I can see, XML
validity isn't 
something that check needs in order to do its business - rather, the validity
checking 
_is_ the business.  On this basis, it probably ought to return a boolean
indicating 
whether the input string is a valid XML document.

If the function is wholly or primarily for the library's internal use, it might
make sense 
to have the checker function just throw the exception that will be passed back
to the 
library user.  But this doesn't seem to be the point in this instance, since
the caller 
swallows the exception and asserts.  It might as well just swallow a boolean
return value 
(or a string return containing an error message) and assert.

And once the module is fixed so that the parser checks as it goes along, I
suppose check 
would remain, but it would be a user function for the odd cases where you just
want to 
check validity and don't want to process the content of the file for whatever
reason.  On 
this basis again, logically it ought to return a pass/fail indication rather
than using 
"expection handling".  But this would be a breaking change....

Stewart.
Feb 08 2013
next sibling parent =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 02/08/2013 09:00 AM, Stewart Gordon wrote:

 Moreover, what do people think about the situation whereby check's sole
 payload is to possibly throw an exception?

 Normally under an exception-handling system, the behaviour of a function
 is: Do something, or if you cannot, then throw an exception.

 But what is the "something" in the case of check? As far as I can see,
 XML validity isn't something that check needs in order to do its
 business - rather, the validity checking _is_ the business. On this
 basis, it probably ought to return a boolean indicating whether the
 input string is a valid XML document.


Absolutely.

It should still throw if the task of checking fails e.g. for no memory 
for internal state.

Ali
Feb 08 2013
prev sibling parent Jacob Carlborg <doob me.com> writes:
On 2013-02-08 18:00, Stewart Gordon wrote:


 But this would be a breaking change....


This can be solved by moving the functionality to a new function, say 
"isValid", which returns a boolean. The implementation of "check" would 
then call "isValid" and throw an exception if it returns false.

-- 
/Jacob Carlborg
Feb 09 2013