cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Polar Humenn <phum...@iona.com>
Subject Re: Checkstyle
Date Wed, 04 Apr 2007 15:48:56 GMT
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 <phumenn@iona.com> 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
>>>>
>>>>     
>>>>         
>>     


Mime
View raw message