Return-Path: Delivered-To: apmail-incubator-cxf-dev-archive@locus.apache.org Received: (qmail 30811 invoked from network); 4 Apr 2007 15:49:19 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 4 Apr 2007 15:49:19 -0000 Received: (qmail 79415 invoked by uid 500); 4 Apr 2007 15:49:16 -0000 Delivered-To: apmail-incubator-cxf-dev-archive@incubator.apache.org Received: (qmail 79379 invoked by uid 500); 4 Apr 2007 15:49:16 -0000 Mailing-List: contact cxf-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: cxf-dev@incubator.apache.org Delivered-To: mailing list cxf-dev@incubator.apache.org Received: (qmail 79294 invoked by uid 99); 4 Apr 2007 15:49:16 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 04 Apr 2007 08:49:15 -0700 X-ASF-Spam-Status: No, hits=-0.0 required=10.0 tests=SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (herse.apache.org: domain of polar.humenn@iona.com designates 65.223.216.181 as permitted sender) Received: from [65.223.216.181] (HELO amereast-smg1.iona.com) (65.223.216.181) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 04 Apr 2007 08:49:06 -0700 Received: from amer-ems1.IONAGLOBAL.COM ([10.65.6.25]) by amereast-smg1.iona.com (Switch-3.1.7/Switch-3.1.7) with ESMTP id l34FlLFQ003556 for ; Wed, 4 Apr 2007 11:47:21 -0400 (EDT) Received: from [10.59.0.40] ([10.59.0.40]) by amer-ems1.IONAGLOBAL.COM with Microsoft SMTPSVC(6.0.3790.1830); Wed, 4 Apr 2007 11:48:44 -0400 Message-ID: <4613C8E8.2020106@iona.com> Date: Wed, 04 Apr 2007 11:48:56 -0400 From: Polar Humenn User-Agent: Thunderbird 1.5.0.10 (X11/20070306) MIME-Version: 1.0 To: cxf-dev@incubator.apache.org Subject: Re: Checkstyle References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-OriginalArrivalTime: 04 Apr 2007 15:48:44.0546 (UTC) FILETIME=[B9C72620:01C776D0] X-Virus-Checked: Checked by ClamAV on apache.org Like I said, there are six ways to Sunday to do everything the way you want it. I just disagree with that/your approach. Cheers, -Polar Glynn, Eoghan wrote: > > > >> -----Original Message----- >> From: Polar Humenn [mailto:phumenn@iona.com] >> Sent: 04 April 2007 14:57 >> To: cxf-dev@incubator.apache.org >> Subject: Re: Checkstyle >> >> I would firmly have to disagree. >> >> You have lots of generated code that are classes, don't use >> interfaces, which people have to use to use the API. But I digress. >> > > > What generated code exactly is in the form of classes (that the user has > to extend)? > > At least for JAX-WS, the main bit of generated code exposed to users is > the SEI, where the "I" stands for interface. > > Clearly I'm not saying don't use classes. You can't program any > executable logic into interfaces. > > What I'm saying is, if the user is to extend or implement some of our > code, give them the option to implement an interface. What's so > controversial or disagreeable about that? > > > >> If I am to use a *class* like: >> >> final class UserPass { >> final String user; >> final String pass; >> UserPass(String u, String p) { >> user = u; pass = p; >> } >> String getUser() { return user; } >> String getPass() { return pass; } >> } >> >> I have a contract with the an operation >> >> UserPass getUserPass(); >> >> That what I get back won't change. It wont deliver different >> things for subsequent calls to getUser or getPass. >> > > > So keep UserPass as a class! > > The point is that the application code wouldn't need to extend UserPass > anyway. > > What they do need to extend is the HttpBasicAuthCredsSupplier. So that > should be an interface. And also possibly an abstract class implementing > it, if it contains any non-trivial bolier-plate code. So that the user > has the choice to extend or implement. > > So assuming HttpBasicAuthCredsSupplier is an interface, then UserPass > could be either an inner interface (if want to keep the inner/outter > scoping), or a stand-alone class (if you want to control the > implementation). Take your choice. > > > >> This allows me to make certain assumptions on the calling >> side that I cannot make if UserPass was forced to be an >> interface. Such as I can cache the object instead of the >> results. And I have some assurance that there are no convert >> channels from it. >> > > Covert channels? > > >> Sure, you can put a certain Abstract logic built "immutable" >> class underneath it, but it doesn't *mean* the same thing. I >> can't *guarantee* immutability from the calling side, if all >> I'm requiring it be is an interface. >> > > > No-one is requiring UserPass to be an interface. As the user would never > have to extend it, it doesn't really matter. > > But you can easily get the immutablility you want even if UserPass is an > inner interface (assuming the inner/outter scoping was more important to > you). As Java strings are themselves immutable, just extract the > username & passwd fields from the returned value and re-wrap in a > trivial immutable UserPass impl. > > On the other hand, if you want to dictate the actual UserPass impl, then > make it an outter class. > > No rocket science. And not worth the time that's already been burned on > this discussion either. > > > >> Nope, I don't buy the argument. >> >> The other thing, I can alleviate memory leaks somewhat >> because if it were an interface and somebody decided to give >> me an implementation of it as an inner class and I cached the >> object I end up keeping a reference to the whole outer class. >> > > > A *static* inner class (like the UserPass in your patch) has no > reference to the outter class. > > Only a *non-static* inner class carries a reference to an instance of > the outter class. > > /Eoghan > > > >> The basic upshot is that you can implement it any number of 6 >> different ways to Sunday but the "contracts" *are* different, >> subtle as they may be. >> >> Cheers, >> -Polar >> >> Glynn, Eoghan wrote: >> >>> Well the mumblings were more like advice to use interfaces *in >>> addition >>> to* abstract classes, not *instead of* abstract classes. >>> >>> Plus some pointers to Java features/idiom like @Override, inner >>> interfaces, composition v. inheritance, that would solve >>> >> some of the >> >>> problems Polar perceives. >>> >>> However there *is* a compelling reason for casting public APIs in >>> terms of interfaces, as opposed to the corresponding >>> >> abstract classes. >> >>> We generally do not want to *force* users to extend our abstract >>> classes and use up their one shot at implementation inheritance in >>> doing so, just to get some boiler-plate code. >>> >>> Example would be Polar's HttpBasicAuthSupplier. That should be an >>> interface, possibly also implemented by an abstract class >>> >> holding the >> >>> boiler-plate logic. A user may want to directly implement the >>> interface themselves if they needed to extend a different >>> >> base class >> >>> for some reason, e.g. DialogBoxHttpBasicAuthSupplier >>> >> extends GUIWidget >> >>> implements HttpBasicAuthSupplier. >>> >>> So the abstract base class is in my view a convenience that >>> >> the user >> >>> may choose to take advantage of, or not, as the case may >>> >> be. But they >> >>> should not be forced to do so. So AbstractWSFeature is >>> >> grand, as long >> >>> there's also a WSFeature interface that the user can choose to >>> implement directly. >>> >>> Sure, users who make that choice would be impacted more by future >>> additions to the interface. But in reality, not all new >>> >> methods would >> >>> have a sensible default impl that extensions of the >>> >> abstract class can >> >>> just pick up without any change. >>> >>> As far as the Abstract* naming convention is concerned, I'm >>> >> not a big >> >>> fan of such rules when forced to change my own class names, >>> >> but once >> >>> you get over that irritation I can see the value of the consistency >>> that the rule brings. So I'm not pushed either way. >>> >>> Hey, it could be a lot worse ... at least we don't have the >>> >> C# IFooBar >> >>> naming scheme for interfaces :) >>> >>> Cheers, >>> Eoghan >>> >>> >>> >>> >>>> -----Original Message----- >>>> From: Dan Diephouse [mailto:dan@envoisolutions.com] >>>> Sent: 04 April 2007 00:50 >>>> To: cxf-dev@incubator.apache.org >>>> Subject: Re: Checkstyle >>>> >>>> I completely agree that we should get rid of this rule. >>>> >>>> First of all, Aegis from XFire use Type which is abstract, and I >>>> don't want to change this for migration reasons which is >>>> >> one of the >> >>>> reasons pmd is disabled for this module. >>>> >>>> Second, I agree that the Abstract/base/factory naming is >>>> >> incredibly >> >>>> awkward. >>>> I hear some mumblings of "just use interfaces" - but >>>> >> abstract classes >> >>>> provide a much more robust way to add features in the >>>> >> future. With an >> >>>> interface, if you add a method, it will break every class that >>>> implemented that interface. With abstract classes if you add a new >>>> method, all you need to do is provide a default >>>> >> implementation of it >> >>>> and everything will work swell in the future. So I tend to use >>>> abstract classes more and more to avoid future incompatibilities >>>> (AbstractWSFeature, AbstractServiceFactoryBean, >>>> AbstractEndpointFactory). With at least the first two, I >>>> >> would like >> >>>> to get ride of the Abstract. >>>> >>>> - Dan >>>> >>>> On 4/3/07, Polar Humenn wrote: >>>> >>>> >>>>> Is there a "good" motivation of why "abstract" classes have to be >>>>> named "Abstract" "Base" or "Factory"? >>>>> >>>>> If a class is declared "abstract", I want the *compiler* to >>>>> >>>>> >>>> tell the >>>> >>>> >>>>> developer that s/he has not filled out a particular >>>>> >>>>> >>>> functionality when >>>> >>>> >>>>> s/he extends the abstract class. Not that I want it named >>>>> >>>>> >>>> "Abstract". >>>> >>>> >>>>> For example, >>>>> >>>>> abstract class Muffin { >>>>> ...... >>>>> abstract Kind kind(); >>>>> } >>>>> >>>>> I really don't want my code littered with method definitions like: >>>>> >>>>> void eat(AbstractMuffin muff) { >>>>> >>>>> I want it to be: >>>>> >>>>> void eat(Muffin muff); >>>>> >>>>> because that's what it is. It's not an AbstractMuffin, it's >>>>> >>>>> >>>> a Muffin! >>>> >>>> >>>>> Can we get rid of that particular checkstyle rule? >>>>> >>>>> I say that, because it forces, either >>>>> >>>>> a) illogical names, like AbstractMuffin, to be used in >>>>> >> definitions, >> >>>>> making for >>>>> awkwardness. (i.e. eat(AbstractMuffin muff); >>>>> >>>>> b) default implementations, just to avoid the illogical names! >>>>> >>>>> This particular avoidance causes errors to be caught >>>>> >>>>> >>>> at run time >>>> >>>> >>>>> instead of compile time, where it should be caught! And >>>>> sometimes causing >>>>> a loss of time to find it. >>>>> >>>>> For example, with the current checkstyle rule I could be >>>>> >> forced to >> >>>>> write the class with a default implementation expecting it to be >>>>> overridden. (Except there is no way to tell a compiler that). >>>>> >>>>> class Muffin { >>>>> ..... >>>>> Kind kind() { >>>>> return Kind.BLUEBERRY; >>>>> } >>>>> } >>>>> >>>>> void eat(Muffin muff) { >>>>> System.out.println("I'm eating a " + muff.kind() + >>>>> >>>>> >>>> " muffin!"); >>>> >>>> >>>>> } >>>>> >>>>> and a developer goes ahead and writes: >>>>> >>>>> class CornMuffin extends Muffin { >>>>> Kind kiend() { >>>>> return Kind.CORN; >>>>> } >>>>> } >>>>> >>>>> and it compiles fine without problems. Subsequently he >>>>> >> can't figure >> >>>>> out why his application still says he has a BLUEBERRY muffin, >>>>> especially when he has used "eat(new CornMuffin())". >>>>> >>>>> This kind of pattern complete adverted the use of compiler >>>>> >>>>> >>>> protections. >>>> >>>> >>>>> Cheers, >>>>> -Polar >>>>> >>>>> >>>>> >>>>> >>>>> >>>> -- >>>> Dan Diephouse >>>> Envoi Solutions >>>> http://envoisolutions.com | http://netzooid.com/blog >>>> >>>> >>>> >>