Introduce an Interface that is implemented by PircBotX

[GoogleCodeExporter]

Hi,

Thanks for this nice rewrite/extension of PircBot! This is great.

To go further in the domain of extensibility, could you refactor the PircBotX 
class so that it extends an Interface?

I need this in my code so that I can provide a Mock implementation for 
PircBotX. Right now I have to do convoluted stuff.

Thanks!

Original issue reported on code.google.com by vmas...@gmail.com on 4 Mar 2012 at 2:56

[GoogleCodeExporter]

An interface for the entire PircBotX class? Sorry, that just doesn't seem 
really useful. I'm even struggling to see why you need to mock the PircBotX 
class, verifying that stuff got sent can be done with basic overriding (look at 
sendRawLine and the other "core" methods)

If you really need a mocked PircBotX, look at some of the unit tests where I 
use mockito. With it you can use spy() for real objects. 

Real reason: An interface for the PircBotX class becomes a maintenance issue, 
code clutter, and another reason to make everything an interface (bad). 

Original comment by Lord.Qua...@gmail.com on 4 Mar 2012 at 3:39

• Changed state: WontFix
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Using mockito or any other mock system (I use jmock2 which also suports mocking 
classes - when it's possible) to mock a class is a code smell. Mocking a class 
is always a code smell and doesn't always work BTW. Some change to the class 
can easily break that.

To be more precise I have 2 use cases:
* Provide a Stub to replace the PircBotX implementation for my functional tests 
so that nothing gets sent over the wire
* XWiki uses a component system and to implement a component I need an 
interface. Right now I have to extract an interface from PircBotX.

Original comment by vmas...@gmail.com on 4 Mar 2012 at 4:18

[GoogleCodeExporter]

Another issue I have is with the Listeners.

Right now they use generics: <T extends PircBotX>

This is blocking me since I have my interface and thus cannot pass it...

This is another good example why PircBotX should be an interface IMO.

Thanks for reconsidering.

Original comment by vmas...@gmail.com on 4 Mar 2012 at 5:15

[GoogleCodeExporter]

The first issue with unit tests can be fixed by overriding sendRawLine and 
optionally sendRawLineNow. You could also fake the OutputThread in 
createOutputThread. 

The second one is more tricky. I don't know anything about the XWiki system, 
but if it uses interfaces then its eventually going to need a concrete class. 
So why not create a wrapper component class with getBot() or something. This 
has the advantage of a) having some setup code and b) not having to maintain 
your own interface. 

Those were just things off the top of my head. 

Original comment by Lord.Qua...@gmail.com on 4 Mar 2012 at 5:30

[GoogleCodeExporter]

Yes this is a good idea but it doesn't solve everything. The biggest issue I 
have now is with the Listeners which have <T extends PircBotX>.

I'm writing a unit test for my listener and I need to mock the concrete class 
PircBotX and that's bad. You can google and you'll find a lot of articles on 
the web explaining why it's a code smell (here's just one: 
http://www.mockobjects.com/2007/04/test-smell-mocking-concrete-classes.html ).

Thanks for the suggestions!

Original comment by vmas...@gmail.com on 5 Mar 2012 at 8:57

[GoogleCodeExporter]

Also note that having a BotManager component with a getBot() method won't help 
at all since getBot() will need to return PircBotX and thus mocking it will 
need to still mock PircBotX.

Original comment by vmas...@gmail.com on 5 Mar 2012 at 12:31

[GoogleCodeExporter]

Found another issue: Channel is a class and not an interface. It's hard to 
mock. As a consequence I need to introduce a class that extend Channel in order 
to be able to create an instance of it... This is another code smell IMO.

Original comment by vmas...@gmail.com on 5 Mar 2012 at 12:43