Page MenuHomePhabricator

Add a `PhutilMethodNotImplementedException` class
ClosedPublic

Authored by joshuaspence on Jul 21 2014, 9:02 AM.
Tags
None
Referenced Files
F14394220: D9992.id24030.diff
Sun, Dec 22, 1:48 AM
Unknown Object (File)
Tue, Dec 10, 1:07 PM
Unknown Object (File)
Wed, Nov 27, 8:36 PM
Unknown Object (File)
Wed, Nov 27, 9:25 AM
Unknown Object (File)
Wed, Nov 27, 9:25 AM
Unknown Object (File)
Wed, Nov 27, 9:24 AM
Unknown Object (File)
Wed, Nov 27, 9:24 AM
Unknown Object (File)
Wed, Nov 27, 9:24 AM
Subscribers

Details

Summary

I think that "Not implemented" is common enough to warrant a dedicated exception class.

Test Plan
[2014-07-21 14:33:42] EXCEPTION: (PhutilMethodNotImplementedException) Method testMethod in class TestClass is not implemented! at [<phutil>/test.php:7]
  #0 TestClass::testMethod() called at [<phutil>/test.php:11]

[2014-07-21 14:33:14] EXCEPTION: (PhutilMethodNotImplementedException) Function foo is not implemented! at [<phutil>/test.php:13]
  #0 foo() called at [<phutil>/test.php:15]

Diff Detail

Repository
rPHU libphutil
Branch
notimplemented
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 1799
Build 1800: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Add a `NotImplementedException` class.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
joshuaspence edited edge metadata.

Move file to src/error

epriestley edited edge metadata.
  • Let's pseudo-namespace this (PhutilNotImplementedException)? It seems likely that it might collide with third-party code.
  • It might be nice to debug_backtrace() and reach into the calling frame to construct a default error message ("method X in class Y is not implemented"). Maybe it should be PhutilMethodNotImplementedException to set up an expectation that it's called from a method body and can reasonably expect to interpret the call stack like that? Although it could probably figure things out reasonably well.
This revision is now accepted and ready to land.Jul 21 2014, 1:43 PM
joshuaspence edited edge metadata.
  • Rename to PhutilMethodNotImplementedException
  • Throw a better default exception message

Like this?

[2014-07-21 14:33:42] EXCEPTION: (PhutilMethodNotImplementedException) Method testMethod in class TestClass is not implemented! at [<phutil>/test.php:7]
  #0 TestClass::testMethod() called at [<phutil>/test.php:11]

[2014-07-21 14:33:14] EXCEPTION: (PhutilMethodNotImplementedException) Function foo is not implemented! at [<phutil>/test.php:13]
  #0 foo() called at [<phutil>/test.php:15]
joshuaspence retitled this revision from Add a `NotImplementedException` class to Add a `PhutilMethodNotImplementedException` class.Jul 21 2014, 2:39 PM
joshuaspence edited the test plan for this revision. (Show Details)

Like this?

[2014-07-21 14:33:42] EXCEPTION: (PhutilMethodNotImplementedException) Method testMethod in class TestClass is not implemented! at [<phutil>/test.php:7]
  #0 TestClass::testMethod() called at [<phutil>/test.php:11]

[2014-07-21 14:33:14] EXCEPTION: (PhutilMethodNotImplementedException) Function foo is not implemented! at [<phutil>/test.php:13]
  #0 foo() called at [<phutil>/test.php:15]

Hmm, since the stack trace already provides the class/method I wonder if there's any point in trying to provide a detailed error message.

There are common cases where the stack trace is not shown:

  • arc without --trace
  • web UI with developer mode off.

Users are also slightly better ("bad") at reporting error messages than stack traces ("very bad").