Talk:Requests for comment/Assert
Personally, I don't like the different variants of assertions. The syntactic sugar of
MWAssert::argument() is nice, but having to decide whether something is a precondition, a postcondition or an invariant feels like work :-). Is
MWAssert::something( $argument * $intermediaryResult < $arbitraryNumber, …) an assertion of the argument, a post-condition of the computation so far or a pre-condition of the following computation? I don't think that being able to raise different exceptions (what to do with them?) is worth it. --Tim Landscheidt 04:30, 25 January 2014 (UTC)
- Perhaps it's not so important to distinguish between pre- and postconditions. I'd like to get some feedback on that question from other people.
- I do think it's nice to have specialized methods for makign assert6ions about parameters, for convenience.
- -- Daniel Kinzler (WMDE) (talk) 14:57, 23 July 2014 (UTC)
Use outside of MediaWiki
Hi Daniel, I think this is an interesting idea (in a good way). The main reservation I have about is mainly a question of naming. Calling it "MWAssert" basically implies that any code that uses it now has a dependency on the MediaWiki monolithic core. Given how little code this is, and how easily this could be used in code with no dependency on MediaWiki (correct?), could we use this to establish a precedent for small modules that are suitable for standalone distribution via Composer or similar means? -- RobLa-WMF (talk) 03:53, 26 January 2014 (UTC)
- @Duesentrieb: Daniel, could you answer this? Thanks! Sharihareswara (WMF) (talk) 01:13, 12 July 2014 (UTC)
- Sure, why not! I could make this a small project on github. One thing I'm unsure about is the "correct" namespace to use. Should it start with "Wikimedia\"? Probably. Should it be "Wikimedia\Assertions\Assert"? That feels redundant. "Wikimedia\Utils\Assert"? That feels too generic, names like "utils" should be avoided... -- Daniel Kinzler (WMDE) (talk) 14:57, 23 July 2014 (UTC)
- Hm... after a bit of thought... I don't know. It would be cool to have this as a separate module, reusable via composer. But MediaWiki core would then have a hard dependency on that external module. MediaWiki would no longer work without composer install, or we would have to bundle the Assert module somehow (and also make it a git submodule or whatever). Are we ready to handle that kind of thing nicely? -- Daniel Kinzler (WMDE) (talk) 12:53, 24 July 2014 (UTC)
This sounds wonderful to me. I wonder if it is worth looking at how Guava did this for some inspiration. In particular they accept a format string and an array of arguments for custom messages in some of their assertion methods. They do this to delay building the message for assertion failures when the assertion doesn't fail. -- — Preceding unsigned comment added by NEverett (WMF) (talk • contribs)
- I'm reluctant to build this into the Assert class. Is building the message string actually that much of a performance issue? But perhaps this can be done simply by allowing any number of extra parameters, which would then be injected into the description message using placeholders like $1 or %s. -- Daniel Kinzler (WMDE) (talk) 14:57, 23 July 2014 (UTC)
It might be nice to have an optional variant that we only execute during development and testing. The Lucene project makes heavy use of something like this (Java's asserts) in performance sensitive code. -- — Preceding unsigned comment added by NEverett (WMF) (talk • contribs)
- I do not know of any way to do this in PHP. The compiler would need to know about the assert (that's what PHP's native assert() is for - unfortunately, it's evil) -- Daniel Kinzler (WMDE) (talk) 14:57, 23 July 2014 (UTC)
- @Aude: @Duesentrieb: - changeset? Also, there are still some questions outstanding above. Thanks! Sharihareswara (WMF) (talk) 01:13, 12 July 2014 (UTC)
The downside of handmade assertions is that function calls are not free, and if you put it in some hotspot (and it's hard to know which code may become hotspot for some scenario), it can cost dearly. OTOH, in production complex asserts will not be that useful anyway.
- OTOH, if we're running hhvm we could make hhvm support it sooner maybe? But it won't work properly on PHP 5.3 I'm afraid.