Bug 109301 - Category.getChildCategories() throws a java.lang.Error
Category.getChildCategories() throws a java.lang.Error
Status: CLOSED RAWHIDE
Product: Red Hat Web Application Framework
Classification: Retired
Component: other (Show other bugs)
nightly
All Linux
medium Severity high
: ---
: ---
Assigned To: Vadim Nasardinov
Jon Orris
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2003-11-06 12:59 EST by Daniel Berrange
Modified: 2007-04-18 12:59 EDT (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-02 13:44:30 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Daniel Berrange 2003-11-06 12:59:56 EST
Description of problem:
A metric tonne of code uses the old java.lang.Collection based APIs in
c.a.categorization.Category, which are in the process of being phased
out in favour of the new persistence-collection based APIs. This old
APIs have been marked deprecated, which is right, but their
implementations have been completely removed & replaced with 

  throw java.lang.Error("deprecated");

So, all usage of these methods, while still compiling just fine,
throws errors at runtime. Common practice when deprecating methods is
to leave the existing impl intact (or provide functionally equiv
implemnentation) for some period of time until client code has
migrated to new APIs. 

If we want to leave the existing APIs in there & mark them deprected
then they should be be made to continue to operate as before. If we
want to force people to migrate to new APIs, then we should remove the
old ones immediately so you get compile errors.

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:
Comment 1 Vadim Nasardinov 2003-11-06 14:59:58 EST
1.  > If we want to leave the existing APIs in there & mark them
    > deprected then they should be be made to continue to operate as
    > before.


2.  > If we want to force people to migrate to new APIs, then we
    > should remove the old ones immediately so you get compile
    > errors.


I would like to suggest a third possibility:

 3. Mark the methods as deprecated AND throw an Error (or
    UnsupportedOperationException).

Option 1 doesn't work in practice, because people routinely ignore
deprecation warnings.

Option 2 is painful, because going down that path produces a metric
shitload of compile-time errors leaving you with a very broken system.

Option 3 addresses both of these scenarios neatly.  It does not
generate compile-time errors, thus sparing clients a lot of hair
pulling and tooth gnashing.  However, it does generate compile-time
warnings for those who care to update their code.  Those who don't
care to update will be forced to do so when they run into runtime
errors.  An indisputably superior approach any way you look at it.

That said, I caved in to pressure and removed all the 'new
Error("deprecated")' clauses in changes 37767 and 37768.
Comment 2 Daniel Berrange 2003-11-06 19:22:34 EST
Urm, Option 3 is what we already had when I entered this ticket & its
lame because you never know what code is broken until you click a link
and get the error 500.

Both Option 2 & 3 result in the same amount of work for the
application developer - namely that they have to change their app to
use the new APIs. Option 2 is preferable because the compile time
errors show exactly where the changes need to be made before you even
try to start a server. With option 3, I could fix 9/10 uses of
getChildCategories, miss one & not find it till 6 months later when
some client happens to execise that piece of code. 

This is not superior in any way. If I hadn't encountered this bug by
accident today we'd have released this 100% broken code for APLAWS
without any incling of the problem. If a piece of code is broken &
needs updating, we need to know about it & fix it ASAP. cf, the
debacle over making setID a no-op & the myriad of subtle bugs we
didn't detect till months later. If setID had been deleted we would
have found all the problem areas immediately.

Comment 3 Daniel Berrange 2003-11-20 06:33:07 EST
This bug has been re-introduced in the categorization land. As noted
previously if methods are going to be left in place as deprecated,
then they should remain fully functional. If not, then they should be
deleted altogether, so that we can quickly identify all code that
requires fixing. Runtime errors are unacceptable.
Comment 4 Vadim Nasardinov 2003-11-20 11:29:40 EST
Fixed in 38168.
Comment 5 David Lawrence 2006-07-17 22:57:48 EDT
QA_READY has been deprecated in favor of ON_QA. Please use ON_QA in the future.
Moving to ON_QA.
Comment 6 Daniel Berrange 2006-09-02 13:44:30 EDT
Closing old tickets

Note You need to log in before you can comment on or make changes to this bug.