New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test protected method #322

Closed
marmotz opened this Issue Mar 29, 2014 · 35 comments

Comments

Projects
None yet
5 participants
@marmotz
Contributor

marmotz commented Mar 29, 2014

Hi,

since the documentation was not up to date and I don't follow recent features of atoum, I would like to know if it is always not possible to test a protected method directly or via a mock or via an other wonderfull technical.

I know this is a delicate question and @mageekguy did not want to implement this feature, but I just want to know if it is possible right now.

If it is still not, I would like to open peaceful discussion about that point because I think this is a convenient feature for all testers.

During our last discussion about that, @mageekguy tell us (on irc chan) that a unit test should test only public API, so only public method.
He asks us why this methods are not public and he suggests to modify method we want to test to public visibility.
In my mind (and I'm not the only one to think like that), as a class developper, I want to expose a public API of my class and "hide" internal methods with protected methods. I use very rarely private visibility because I think that all can be modified by inheritance. (But for that point, it's my only point of view and, this is not the subject of this issue).
So, I want to test my public API and atoum offer me all the tools I need to do that (and very big thank you about that !).
But, sometimes (frequently, in fact), I want to test an internal method because it contains some sensible piece of code.

In my mind, I could compare public/protected tests with functional/unit tests.
If I functionaly test only my application, it's good, but I only test the results and I can't be "sure" that all the internal bricks work. So i unit test.
If I test only public methods, it's good, but I only test the results again and I can't be "sure" that all the internal bricks work...

I don't want to use reflection to change visibility, it's a very dirty hack which cause much problem that it resolves. I thought about automatic/magic "test{methodName}" methods who call parent "methodName" method and return result that we could test with all wonderful tools that offer atoum.

Because I do not have much hope to see this feature added, what technical solution I can use to add this feature in my atoum configuration. In another words, how can create "plugins" to atoum ? (Maybe, I should open a new ticket for this last question ?)

@guiled

This comment has been minimized.

Show comment
Hide comment
@guiled

guiled Mar 29, 2014

Contributor

The main argument against doing unit test on non-public method is that their code must be tested through public method.

If your protected code is complex and important enough to be tested, it can be tested through public method. All you have to do is to make your tests work every cases your protected code needs in order to be executed by TU (ie to be tested).

If it is impossible to trigger some protected code through public method, it could be a clue that your protected code is useless (because unused / unusable)

Nonetheless, it could be possible to make a king of "plugin" to atoum that will override some classes to make protected method testable with atoum. I'm sure it would be possible because atoum has been built to provide this king of behaviour changes. Then it would be an unofficial plugin that those who want to test protected method would download, install and use in their project (like we can do so many other projects or software...)

Contributor

guiled commented Mar 29, 2014

The main argument against doing unit test on non-public method is that their code must be tested through public method.

If your protected code is complex and important enough to be tested, it can be tested through public method. All you have to do is to make your tests work every cases your protected code needs in order to be executed by TU (ie to be tested).

If it is impossible to trigger some protected code through public method, it could be a clue that your protected code is useless (because unused / unusable)

Nonetheless, it could be possible to make a king of "plugin" to atoum that will override some classes to make protected method testable with atoum. I'm sure it would be possible because atoum has been built to provide this king of behaviour changes. Then it would be an unofficial plugin that those who want to test protected method would download, install and use in their project (like we can do so many other projects or software...)

@marmotz

This comment has been minimized.

Show comment
Hide comment
@marmotz

marmotz Mar 29, 2014

Contributor

I understand this argument, but I should tell you "Don't unit test your code, just functionally test your app with all input possibility and all your code should be executed at least one time". It's true, but not very practical, you see ? :)

Contributor

marmotz commented Mar 29, 2014

I understand this argument, but I should tell you "Don't unit test your code, just functionally test your app with all input possibility and all your code should be executed at least one time". It's true, but not very practical, you see ? :)

@guiled

This comment has been minimized.

Show comment
Hide comment
@guiled

guiled Mar 29, 2014

Contributor

It sounds to me that you don't want to consider all different situations in public methods for testing all different cases in protected methods, but to test all different cases of a protected methods. I'm not really sure this is consistent.
Of course I might be wrong

Contributor

guiled commented Mar 29, 2014

It sounds to me that you don't want to consider all different situations in public methods for testing all different cases in protected methods, but to test all different cases of a protected methods. I'm not really sure this is consistent.
Of course I might be wrong

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 30, 2014

Member

Hi all,

from time to time I do have the need to test protected method.

Why so ? Because as a developper, I provide base classes for others to
extend. I can't exactly remember my last use case... but I think it was
something like an "auto-cached" kind of component (this pecular example was
violating the single principle responsability, but this is not the point).

I really needed to test the protected method because the class I was
writing at the time was supposed to be abstract, therefore, her only
purpose as a class was to provide its protected methods to its children.

How have I done this ? Well, basically, I've cheated.

I've extended the abstract class and made all the protected methods I was
interested in public, and then I was obviously able to test them as usual.

It basically sucked not having an official way of doing this with atoum, it
could have saved me valuable time, but I understand the points when it
comes to state that this is an edge case.

gerald

On Sat, Mar 29, 2014 at 11:06 PM, guiled notifications@github.com wrote:

It sounds to me that you don't want to consider all different situations
in public methods for testing all different cases in protected methods, but
to test all different cases of a protected methods. I'm not really sure
this is consistent.

Reply to this email directly or view it on GitHubhttps://github.com/atoum/atoum/issues/322#issuecomment-39010645
.

Member

geraldcroes commented Mar 30, 2014

Hi all,

from time to time I do have the need to test protected method.

Why so ? Because as a developper, I provide base classes for others to
extend. I can't exactly remember my last use case... but I think it was
something like an "auto-cached" kind of component (this pecular example was
violating the single principle responsability, but this is not the point).

I really needed to test the protected method because the class I was
writing at the time was supposed to be abstract, therefore, her only
purpose as a class was to provide its protected methods to its children.

How have I done this ? Well, basically, I've cheated.

I've extended the abstract class and made all the protected methods I was
interested in public, and then I was obviously able to test them as usual.

It basically sucked not having an official way of doing this with atoum, it
could have saved me valuable time, but I understand the points when it
comes to state that this is an edge case.

gerald

On Sat, Mar 29, 2014 at 11:06 PM, guiled notifications@github.com wrote:

It sounds to me that you don't want to consider all different situations
in public methods for testing all different cases in protected methods, but
to test all different cases of a protected methods. I'm not really sure
this is consistent.

Reply to this email directly or view it on GitHubhttps://github.com/atoum/atoum/issues/322#issuecomment-39010645
.

@marmotz

This comment has been minimized.

Show comment
Hide comment
@marmotz

marmotz Mar 30, 2014

Contributor

@guiled I have a problem with your suggestion because I do want to do unit test, so I want to test unitarily my code.
Call public method to reach a far protected method (you don't how many level I have between the public method and this protected method) seems to me very hazardous, not practical, very complicated to implement and absolutely not maintainable (I talk about tests, not my code)

@geraldcroes is right, I have same problem. When I create abstract class with protected method, I need to test this protected method and no, I don't want to make them public.

Contributor

marmotz commented Mar 30, 2014

@guiled I have a problem with your suggestion because I do want to do unit test, so I want to test unitarily my code.
Call public method to reach a far protected method (you don't how many level I have between the public method and this protected method) seems to me very hazardous, not practical, very complicated to implement and absolutely not maintainable (I talk about tests, not my code)

@geraldcroes is right, I have same problem. When I create abstract class with protected method, I need to test this protected method and no, I don't want to make them public.

@jubianchi

This comment has been minimized.

Show comment
Hide comment
@jubianchi

jubianchi Mar 30, 2014

Member

Hi all !

I understand you concern, in your use cases, testing protected methods is a need. But, as it was said in the thread, you can test protected method through public ones (this is the first argument against testing protected methods)

the second point is the following : atoum was made to enforce TDD. TDD principles say that you should write a failing test and then, ou have to write as less code as possible to make it green. Once done, you refactor. Protected method are often created during the refactor phase so their code will likely already be tested.

Protected method are implementation details.

We can also think about extracting those protected methods to classes providing a public API to test this particular code. To me, this has the advantage of forcing classes writer to respect SRP. To me, if you have sensible code in protected methods, it is because your protected are doing a valuable job. If it is hidden in protected method in your class, it's perhaps because they are not directly related to the class' role (i.e they are doing a job helping by providing tools, helpers, ...).

For example, a class abstracting some kind of configuration with protected method to create/write the config. file. The file related job will likely be in protected method, but is it the good place ? I think not. I think this should be delegated to another component providing some public API.

I really understand you concerns but I also don't know what's the best :

  • Doing tricky test to be able to test protected methods (like @geraldcroes did)
  • Making protected method visible in tests (with reflection tools)
  • Not testing protected directly but through public methods

I think both solutions have their advantages but also have drawbacks.

Member

jubianchi commented Mar 30, 2014

Hi all !

I understand you concern, in your use cases, testing protected methods is a need. But, as it was said in the thread, you can test protected method through public ones (this is the first argument against testing protected methods)

the second point is the following : atoum was made to enforce TDD. TDD principles say that you should write a failing test and then, ou have to write as less code as possible to make it green. Once done, you refactor. Protected method are often created during the refactor phase so their code will likely already be tested.

Protected method are implementation details.

We can also think about extracting those protected methods to classes providing a public API to test this particular code. To me, this has the advantage of forcing classes writer to respect SRP. To me, if you have sensible code in protected methods, it is because your protected are doing a valuable job. If it is hidden in protected method in your class, it's perhaps because they are not directly related to the class' role (i.e they are doing a job helping by providing tools, helpers, ...).

For example, a class abstracting some kind of configuration with protected method to create/write the config. file. The file related job will likely be in protected method, but is it the good place ? I think not. I think this should be delegated to another component providing some public API.

I really understand you concerns but I also don't know what's the best :

  • Doing tricky test to be able to test protected methods (like @geraldcroes did)
  • Making protected method visible in tests (with reflection tools)
  • Not testing protected directly but through public methods

I think both solutions have their advantages but also have drawbacks.

@marmotz

This comment has been minimized.

Show comment
Hide comment
@marmotz

marmotz Mar 30, 2014

Contributor

Well, my code respect SRP and protected methods have no reason to export to an other class or become public.

I hope I'm wrong, but what I understand of what @guiled and @jubianchi (and @mageekguy) said is: "you must (should ?) adapt your code to the tester tool" and I'm very surprised you can tell things like this.

It's not all the time, but I (and @geraldcroes too) have some case or I need to test protected method.
Yes, sure, I can't test it through public method, but it's painful and if there are many level between this 2 methods, be sure that it's very not maintainable. And, I think it's not the "unit test spirit" because I don't unit test this particular method. I'll never have the result of this method, I'll just "know" if the public method works for all tests I wrote.

Nevertheless, you should add a fourth solution: generate a public method that calls protected method by passing all arguments and returns result:

  • existing code continue to work normally, no strange behavior
  • protected would be virtually visible only for test purpose via mock (except if you are an ugly dirty coder and use atoum mock in your application... but it's already possible without atoum)
Contributor

marmotz commented Mar 30, 2014

Well, my code respect SRP and protected methods have no reason to export to an other class or become public.

I hope I'm wrong, but what I understand of what @guiled and @jubianchi (and @mageekguy) said is: "you must (should ?) adapt your code to the tester tool" and I'm very surprised you can tell things like this.

It's not all the time, but I (and @geraldcroes too) have some case or I need to test protected method.
Yes, sure, I can't test it through public method, but it's painful and if there are many level between this 2 methods, be sure that it's very not maintainable. And, I think it's not the "unit test spirit" because I don't unit test this particular method. I'll never have the result of this method, I'll just "know" if the public method works for all tests I wrote.

Nevertheless, you should add a fourth solution: generate a public method that calls protected method by passing all arguments and returns result:

  • existing code continue to work normally, no strange behavior
  • protected would be virtually visible only for test purpose via mock (except if you are an ugly dirty coder and use atoum mock in your application... but it's already possible without atoum)
@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

Hi,

@julienbianchi

"you can test protected method through public ones (this is the first argument against testing protected methods)"

I'm OK with this. I'm not really asking for a way to test protected method
"as is", I'm wondering if there could be a way to ease the path to test
protected method by magically making them available through any mean,
embedded in atoum.

the second point is the following : atoum was made to enforce TDD. TDD principles say that you should write a failing test and then, ou have to write as less code as possible to make it green. Once done, you refactor. Protected method are often created during the refactor phase so their code will likely already be tested.

Refactoring is the exact use case that often leads me to the need of
testing protected methods. Although, sometimes I have to refactor stuff
beyond the class itself

Protected method are implementation details.

I do agree with this statement in 90 percent of the time, but sometimes, I
do not.

I do not agree with this when it comes to see my protected methods as being
"a contract" I'm taking responsibilities for, made available to other's own
classes that will eventually extend my former ones.

When I'm writing a private method, I say "this is implementation detail,
move on".

When I'm writing a protected method, I say "Hey, this is not the class
goal... but if you want to extend the class, you may look it over as you
could benefit from the internal service I provide here".

Let's take an example... a pricing class with a very basic algorithm. "If
you're younger than 18 => then 2 euros, if not, 4 euros".

interface Pricing {
    public function getPriceFor(Order $order);
}

class BasicPricing implements Pricing {
   public function getPriceFor(Order $order) {
      if ($this->isYoungerThan18($order->getPersonn())) {
          return 2;
      } else {
           return 4;
      }
  }
   private isYoungerThan18 (Personn $personn) {
       //algorithm to check if < 18
   }
}

Ok. If I had written all of this the TDD way, I wouldn't have had any
problem, whether my isYounger function was private or protected.

Now, the business department comes with a new pricing formula : [If we're
in the middle of the summer, I'll also have to consider "other stuffs" to
calculate the price].

Here, I decide to implement two different algorithm, one for the summer
time, one for the remaining period of the year. It will be up to the
PricingFactory to decide wether I need the Summer implementation or not.

I don't want to duplicate code in two separate classes to check for the age
of the person. I also want to keep the basic algorithm in a single place.
I don't want to come with an over thought solution, with classes and
interfaces assuming responsibilities that don't even exist in the business,
or so business specific their reuse value is next to zero. So I'll
basically end up with a "template method" pattern.

Its only responsability is to provide root algorithm and basic services
that fulfill its own algorithm and "maybe" its children algorithm. (that
is, the "isYounger" part).

I do want to test this, and I want to test this right in its own dedicated
test.

I really understand you concerns but I also don't know what's the best [...]

Me neither. I've just stated that it can be a painfull process to subclass
a fake child of the class in my test.

Maybe an embedded helper would be fine ?

Maybe a syntax to "exposeProtected", like

$this->exposeProtected('myFunction') (on a side note, we could also think
of $this->implementAbstract ('myFunction', function(){});

I just don't know, I basically wanted to take part in the conversation as I
face some use cases (edge cases amidst hundreds of regular cases).

gerald

Member

geraldcroes commented Mar 31, 2014

Hi,

@julienbianchi

"you can test protected method through public ones (this is the first argument against testing protected methods)"

I'm OK with this. I'm not really asking for a way to test protected method
"as is", I'm wondering if there could be a way to ease the path to test
protected method by magically making them available through any mean,
embedded in atoum.

the second point is the following : atoum was made to enforce TDD. TDD principles say that you should write a failing test and then, ou have to write as less code as possible to make it green. Once done, you refactor. Protected method are often created during the refactor phase so their code will likely already be tested.

Refactoring is the exact use case that often leads me to the need of
testing protected methods. Although, sometimes I have to refactor stuff
beyond the class itself

Protected method are implementation details.

I do agree with this statement in 90 percent of the time, but sometimes, I
do not.

I do not agree with this when it comes to see my protected methods as being
"a contract" I'm taking responsibilities for, made available to other's own
classes that will eventually extend my former ones.

When I'm writing a private method, I say "this is implementation detail,
move on".

When I'm writing a protected method, I say "Hey, this is not the class
goal... but if you want to extend the class, you may look it over as you
could benefit from the internal service I provide here".

Let's take an example... a pricing class with a very basic algorithm. "If
you're younger than 18 => then 2 euros, if not, 4 euros".

interface Pricing {
    public function getPriceFor(Order $order);
}

class BasicPricing implements Pricing {
   public function getPriceFor(Order $order) {
      if ($this->isYoungerThan18($order->getPersonn())) {
          return 2;
      } else {
           return 4;
      }
  }
   private isYoungerThan18 (Personn $personn) {
       //algorithm to check if < 18
   }
}

Ok. If I had written all of this the TDD way, I wouldn't have had any
problem, whether my isYounger function was private or protected.

Now, the business department comes with a new pricing formula : [If we're
in the middle of the summer, I'll also have to consider "other stuffs" to
calculate the price].

Here, I decide to implement two different algorithm, one for the summer
time, one for the remaining period of the year. It will be up to the
PricingFactory to decide wether I need the Summer implementation or not.

I don't want to duplicate code in two separate classes to check for the age
of the person. I also want to keep the basic algorithm in a single place.
I don't want to come with an over thought solution, with classes and
interfaces assuming responsibilities that don't even exist in the business,
or so business specific their reuse value is next to zero. So I'll
basically end up with a "template method" pattern.

Its only responsability is to provide root algorithm and basic services
that fulfill its own algorithm and "maybe" its children algorithm. (that
is, the "isYounger" part).

I do want to test this, and I want to test this right in its own dedicated
test.

I really understand you concerns but I also don't know what's the best [...]

Me neither. I've just stated that it can be a painfull process to subclass
a fake child of the class in my test.

Maybe an embedded helper would be fine ?

Maybe a syntax to "exposeProtected", like

$this->exposeProtected('myFunction') (on a side note, we could also think
of $this->implementAbstract ('myFunction', function(){});

I just don't know, I basically wanted to take part in the conversation as I
face some use cases (edge cases amidst hundreds of regular cases).

gerald

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

In this case, don't use heritage, use composition.
If you do TDD, you will see very early that you can not test easily the class behavior without a dependency…

namespace pricing;

interface definition {
    public function setEngine(Engine $engine = null);
    public function getPriceFor(Order $order);
}

namespace;

class pricing implements pricing\definition {
   private $engine = null;

   public function __construct()
   {
      $this->setEngine();
   }

   public function setEngine(Engine $engine = null) {
       $this->engine = $engine ?: new Engine();
   }

   public function getPriceFor(Order $order) {
      return $this->engine->getPrice($order);
   }    
}
Contributor

mageekguy commented Mar 31, 2014

In this case, don't use heritage, use composition.
If you do TDD, you will see very early that you can not test easily the class behavior without a dependency…

namespace pricing;

interface definition {
    public function setEngine(Engine $engine = null);
    public function getPriceFor(Order $order);
}

namespace;

class pricing implements pricing\definition {
   private $engine = null;

   public function __construct()
   {
      $this->setEngine();
   }

   public function setEngine(Engine $engine = null) {
       $this->engine = $engine ?: new Engine();
   }

   public function getPriceFor(Order $order) {
      return $this->engine->getPrice($order);
   }    
}
@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

Reason why I was afraid to put an example here, the "use composition"
answer.

I tried to avoid this by writing : "I don't want to come with an over
thought solution, with classes and interfaces with responsibilities that
don't even exist in the business, or so business specific their reuse value
is next to zero."

You really would consider components that basically check if someone is
under the age of 18 ? This sounds pretty ridiculous to me (no offense).

To take another even more ridiculous example, let's imagine methods for the
sake of readability, that could also be exposed to children :

isInSummer()
getMenAndWomensAgesAppart()
beforeThe4OfJuly()
...

class MyPricingSystem(TellMeIfInSummer $summer, TellMeMenAndWomenAgesAppart
$tmmawaa, TellMeIfBeforeThe4OfJuly $...).... definitely not an option.

These methods make sense in their own scope, but I won't ever ever (ever)
need any component that only check if we are "beforeThe4OfJuly"

I could argue on more specific examples related to a current project, but
this is not really the point. There are situations where composition is not
the best choice, period.

On Mon, Mar 31, 2014 at 11:47 AM, Frédéric Hardy
notifications@github.comwrote:

In this case, don't use heritage, use composition.
If you do TDD, you will see very early that you can not test easily the
class behavior without a dependency...

namespace pricing;

interface definition {
public function setEngine(Engine $engine = null);
public function getPriceFor(Order $order);
}

namespace;

class pricing implements pricing\definition {
private $engine = null;

public function __construct()
{
$this->setEngine();
}

public function setEngine(Engine $engine = null) {
$this->engine = $engine ?: new Engine();
}

public function getPriceFor(Order $order) {
return $this->engine->getPrice($order->getPersonn());
}
}

Reply to this email directly or view it on GitHubhttps://github.com/atoum/atoum/issues/322#issuecomment-39070820
.

Member

geraldcroes commented Mar 31, 2014

Reason why I was afraid to put an example here, the "use composition"
answer.

I tried to avoid this by writing : "I don't want to come with an over
thought solution, with classes and interfaces with responsibilities that
don't even exist in the business, or so business specific their reuse value
is next to zero."

You really would consider components that basically check if someone is
under the age of 18 ? This sounds pretty ridiculous to me (no offense).

To take another even more ridiculous example, let's imagine methods for the
sake of readability, that could also be exposed to children :

isInSummer()
getMenAndWomensAgesAppart()
beforeThe4OfJuly()
...

class MyPricingSystem(TellMeIfInSummer $summer, TellMeMenAndWomenAgesAppart
$tmmawaa, TellMeIfBeforeThe4OfJuly $...).... definitely not an option.

These methods make sense in their own scope, but I won't ever ever (ever)
need any component that only check if we are "beforeThe4OfJuly"

I could argue on more specific examples related to a current project, but
this is not really the point. There are situations where composition is not
the best choice, period.

On Mon, Mar 31, 2014 at 11:47 AM, Frédéric Hardy
notifications@github.comwrote:

In this case, don't use heritage, use composition.
If you do TDD, you will see very early that you can not test easily the
class behavior without a dependency...

namespace pricing;

interface definition {
public function setEngine(Engine $engine = null);
public function getPriceFor(Order $order);
}

namespace;

class pricing implements pricing\definition {
private $engine = null;

public function __construct()
{
$this->setEngine();
}

public function setEngine(Engine $engine = null) {
$this->engine = $engine ?: new Engine();
}

public function getPriceFor(Order $order) {
return $this->engine->getPrice($order->getPersonn());
}
}

Reply to this email directly or view it on GitHubhttps://github.com/atoum/atoum/issues/322#issuecomment-39070820
.

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

Definitely not agree, period ;).
More seriously, you're typically in the use case of the strategy pattern, and composition is the right way to implement it.
Moreover, a price is a price, i.e. it's a float associated to a unit, and it's related to an order, and i'm pretty sure that it's not its responsibility to compute itself (S of SOLID).

Contributor

mageekguy commented Mar 31, 2014

Definitely not agree, period ;).
More seriously, you're typically in the use case of the strategy pattern, and composition is the right way to implement it.
Moreover, a price is a price, i.e. it's a float associated to a unit, and it's related to an order, and i'm pretty sure that it's not its responsibility to compute itself (S of SOLID).

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

Man, I cannot tell how wrong I think you are, You've just overlooked my example and its explanations (Or I've just failed to make my points clear).

Had you looked into the example you would have seen I did use the strategy pattern, and that I did use a TDD approach.

Member

geraldcroes commented Mar 31, 2014

Man, I cannot tell how wrong I think you are, You've just overlooked my example and its explanations (Or I've just failed to make my points clear).

Had you looked into the example you would have seen I did use the strategy pattern, and that I did use a TDD approach.

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

BTW, I don't think that you don't agree (and never will), I think you still haven't taken the time to properly look into the issue exposed here.

Member

geraldcroes commented Mar 31, 2014

BTW, I don't think that you don't agree (and never will), I think you still haven't taken the time to properly look into the issue exposed here.

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

Moreover, a price is a price, i.e. it's a float associated to a unit, and it's related to an order, and i'm pretty sure that it's not its responsibility to compute itself (S of SOLID).

I've never stated the opposite. My "BasicPricing" service calculates the price (float) of an Order.

Please try to consider I know the basics of OOP for the sake of efficiency in our debate.

Member

geraldcroes commented Mar 31, 2014

Moreover, a price is a price, i.e. it's a float associated to a unit, and it's related to an order, and i'm pretty sure that it's not its responsibility to compute itself (S of SOLID).

I've never stated the opposite. My "BasicPricing" service calculates the price (float) of an Order.

Please try to consider I know the basics of OOP for the sake of efficiency in our debate.

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

It appears I have some free time waiting for my delivery process to complete, I'll then give additional input on the compose versus inheritance issue.

Let's consider a Human being. Let's figure out there are Men and Womens. No, Men are not "composed" by humans, they "are".

If both can walk, they both walk the way human walk.... they are not using a "walking engine"... they just walk the way they walk.

I they have hearts, lungs, arms, legs and stuff... I shall choose not to write it down in the source code and consider this "as a fact" in a human being.

Of course there is a drawback : If I want to change some human's heart behaviour, it will require some serious patching in the source code.... as it will in real life.

(EDITED the example to be more specific)

Member

geraldcroes commented Mar 31, 2014

It appears I have some free time waiting for my delivery process to complete, I'll then give additional input on the compose versus inheritance issue.

Let's consider a Human being. Let's figure out there are Men and Womens. No, Men are not "composed" by humans, they "are".

If both can walk, they both walk the way human walk.... they are not using a "walking engine"... they just walk the way they walk.

I they have hearts, lungs, arms, legs and stuff... I shall choose not to write it down in the source code and consider this "as a fact" in a human being.

Of course there is a drawback : If I want to change some human's heart behaviour, it will require some serious patching in the source code.... as it will in real life.

(EDITED the example to be more specific)

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

Both can walk… sure?
How to handle amputation, paralysis, prothesis, wheel chair, Steve Austin and their bionic muscles and eye?
Man and woman are not all the same, each of them as different physical and intellectual capacity, so they are composition of several sub-component (muscle, brain, organ, and so on) with different capacity according to genetic, environment, accident and so on.
And if you want to change a human's heart or patch its behavior, just add a pacemaker or remove it and put in place an another biological or artificial heart, no need to change the rules which drive the body, because the new heart has its own rules, independent of the whole system and just need an interface (nerves, veins) to dialog with it.
It's real life.

Contributor

mageekguy commented Mar 31, 2014

Both can walk… sure?
How to handle amputation, paralysis, prothesis, wheel chair, Steve Austin and their bionic muscles and eye?
Man and woman are not all the same, each of them as different physical and intellectual capacity, so they are composition of several sub-component (muscle, brain, organ, and so on) with different capacity according to genetic, environment, accident and so on.
And if you want to change a human's heart or patch its behavior, just add a pacemaker or remove it and put in place an another biological or artificial heart, no need to change the rules which drive the body, because the new heart has its own rules, independent of the whole system and just need an interface (nerves, veins) to dialog with it.
It's real life.

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

You're just kidding here... and I think that one more time you did not pay attention to what I had just said (with the joke on the "patching the heart").

Member

geraldcroes commented Mar 31, 2014

You're just kidding here... and I think that one more time you did not pay attention to what I had just said (with the joke on the "patching the heart").

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

Definitely need to discuss about that around a beer in real life ;).

Contributor

mageekguy commented Mar 31, 2014

Definitely need to discuss about that around a beer in real life ;).

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

To be exhaustive in my answer, I do agree that humans are different and that some may be crippled... but you do not have to write every possibilities down into your program. The program needs to know what it has to handle, no more. Everything else is an exception, else you'll end up with infinite composition (arm => muscles, blood => white and red blood cells, ...).

Definitely need to discuss about that around a beer in real life ;).

I agree, even if it may take a lot of beers to open your mind to the idea ^^

Member

geraldcroes commented Mar 31, 2014

To be exhaustive in my answer, I do agree that humans are different and that some may be crippled... but you do not have to write every possibilities down into your program. The program needs to know what it has to handle, no more. Everything else is an exception, else you'll end up with infinite composition (arm => muscles, blood => white and red blood cells, ...).

Definitely need to discuss about that around a beer in real life ;).

I agree, even if it may take a lot of beers to open your mind to the idea ^^

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

In your case, the program need to handle several different business rules (< 18, now is before july 4, and so on).
If you use composition to handle them, you don't need to define every possibility, you just need to defined your current business rule.
And when you need to add a new one, just create the corresponding class and inject an instance of it in your pricing instance without update its code.
I'm agree that to do that, you should create a class for each business rule.
But what is the problem with that?
These rules are part of your domain, so it's not a problem.
Their reuse value is next to zero? And? Are you sure? Have you got a crystal ball? And maybe it's true, but you can add new rules easily or use several of them at the same time without any problem, so the fact that these classes are not reusable is not relevant for me.

Contributor

mageekguy commented Mar 31, 2014

In your case, the program need to handle several different business rules (< 18, now is before july 4, and so on).
If you use composition to handle them, you don't need to define every possibility, you just need to defined your current business rule.
And when you need to add a new one, just create the corresponding class and inject an instance of it in your pricing instance without update its code.
I'm agree that to do that, you should create a class for each business rule.
But what is the problem with that?
These rules are part of your domain, so it's not a problem.
Their reuse value is next to zero? And? Are you sure? Have you got a crystal ball? And maybe it's true, but you can add new rules easily or use several of them at the same time without any problem, so the fact that these classes are not reusable is not relevant for me.

@guiled

This comment has been minimized.

Show comment
Hide comment
@guiled

guiled Mar 31, 2014

Contributor

I personally think that a plugin/extension could be helpful in this situation.
Then if one day this plugin appears to be used most of the time with atoum, and is coded following the atoum cc, it could be integrated in atoum itself, like in any plugin life...

Contributor

guiled commented Mar 31, 2014

I personally think that a plugin/extension could be helpful in this situation.
Then if one day this plugin appears to be used most of the time with atoum, and is coded following the atoum cc, it could be integrated in atoum itself, like in any plugin life...

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

In your case, the program need to handle several different business rules (< 18, now is before july 4, and so on).

No, the program needs a pricing system for an order. The system does not care about how the pricing system gets its prices. There is a subtle yet important difference.

If you use composition to handle them, you don't need to define every possibility, you just need to defined your current business rule.

So basically, you're telling me that instead of writing this

<?php 
class DisneyLandParisPricingSystem implements TicketPricingSystem
{
    public function getPrice (Personn $personn)
    {
        if (($personn->getDateOfBirth() - date('Ymd')) <= 2) {
            return 0;
        } elseif (($personn->getDateOfBirth() - date('Ymd')) <= 4) {
            return 10;
        } else {
            return 30;
        }
    }
}

I should write this (simplified for the sake of the example)

class DisneyLandParisPricingSystem implements TicketPricingSystem
{
    protected $utao2;
    protected $utao4;

    public function __construct (UnderTheAgeOf2 $utao2, UnderTheAgeOf4 $utao4)
    {
         $this->utao2 = $utao2;
         $this->utao4 = $utao4;
    }

    public function getPrice (Personn $personn)
    {
        if ($this->utao2->isUnderTheAgeOf2($personn)) {
            return 0;
        } elseif ($this->utao4->isUnderTheAgeOf4($personn)) {
            return 10;
        } else {
            return 30;
        }
    }
}

class UnderTheAgeOf2UsingOccidentalCalendarDateRepresentations implements UnderTheAgeOf2 {
    protected $dateDiffCalculator;
    public function __construct (DateTDiffCalculator $dateDiffCalculator, GetTodayFactory $todayDateFactory)
    {
        $this->dateDiffCalculator = $dateDiffCalculator;
        $this->todayDateFactory = $todayDateFactory;
    }

    public function isUnderTheAgeOf2 (Personn $personn) {
        return $this->dateDiffCalculator->getDifferenceInYears($this->todayDateFactory->createToday(), $personn->getDateOfBirth()) < 2;
    }
}

class UnderTheAgeOf4UsingOccidentalCalendarDateRepresentations implements UnderTheAgeOf4 {
    protected $dateDiffCalculator;
    public function __construct (DateTDiffCalculator $dateDiffCalculator, GetTodayFactory $todayDateFactory)
    {
        $this->dateDiffCalculator = $dateDiffCalculator;
        $this->todayDateFactory = $todayDateFactory;
    }

    public function isUnderTheAgeOf4 (Personn $personn) {
        return $this->dateDiffCalculator->getDifferenceInYears($this->todayDateFactory->createToday(), $personn->getDateOfBirth()) < 4;
    }
}

class DateDiffCalculatorInCivilYear implements DateDiffCalculatorInYear
{
    public function getDifferencesInYears (Date $one, Date $two)
    {
        return $one - $two;
    }
}

I say "You cannot be serious about this", this is object oriented crap if you don't need to reuse DateDiffCalculatorInCivilYear.

These rules are part of your domain, so it's not a problem.

No, these rules are implementation details my domain do not care about. My domain needs a pricing system. If I was writing a pricing system, I would write an ruled based engine to get prices.

Their reuse value is next to zero? And? Are you sure? Have you got a crystal ball?

No, I'm not a seer nor an expert in pricing system. That is the exact reason why I DO NOT dare to write a system that tries to expect what the future will be like.

I do not need "as of today" those components, so I do not take the time to write them, why would I ?
I would be wasting my time doing so.

In a TDD approach, all I have to care about is getting 2 with a baby, 10 for a child and 30 for others.

"If", in the future, it shows that I "should" reuse the component, then and only then should I refactor the code and create a component.

so the fact that these classes are not reusable is not relevant for me.

Good luck writing the whole world !

Member

geraldcroes commented Mar 31, 2014

In your case, the program need to handle several different business rules (< 18, now is before july 4, and so on).

No, the program needs a pricing system for an order. The system does not care about how the pricing system gets its prices. There is a subtle yet important difference.

If you use composition to handle them, you don't need to define every possibility, you just need to defined your current business rule.

So basically, you're telling me that instead of writing this

<?php 
class DisneyLandParisPricingSystem implements TicketPricingSystem
{
    public function getPrice (Personn $personn)
    {
        if (($personn->getDateOfBirth() - date('Ymd')) <= 2) {
            return 0;
        } elseif (($personn->getDateOfBirth() - date('Ymd')) <= 4) {
            return 10;
        } else {
            return 30;
        }
    }
}

I should write this (simplified for the sake of the example)

class DisneyLandParisPricingSystem implements TicketPricingSystem
{
    protected $utao2;
    protected $utao4;

    public function __construct (UnderTheAgeOf2 $utao2, UnderTheAgeOf4 $utao4)
    {
         $this->utao2 = $utao2;
         $this->utao4 = $utao4;
    }

    public function getPrice (Personn $personn)
    {
        if ($this->utao2->isUnderTheAgeOf2($personn)) {
            return 0;
        } elseif ($this->utao4->isUnderTheAgeOf4($personn)) {
            return 10;
        } else {
            return 30;
        }
    }
}

class UnderTheAgeOf2UsingOccidentalCalendarDateRepresentations implements UnderTheAgeOf2 {
    protected $dateDiffCalculator;
    public function __construct (DateTDiffCalculator $dateDiffCalculator, GetTodayFactory $todayDateFactory)
    {
        $this->dateDiffCalculator = $dateDiffCalculator;
        $this->todayDateFactory = $todayDateFactory;
    }

    public function isUnderTheAgeOf2 (Personn $personn) {
        return $this->dateDiffCalculator->getDifferenceInYears($this->todayDateFactory->createToday(), $personn->getDateOfBirth()) < 2;
    }
}

class UnderTheAgeOf4UsingOccidentalCalendarDateRepresentations implements UnderTheAgeOf4 {
    protected $dateDiffCalculator;
    public function __construct (DateTDiffCalculator $dateDiffCalculator, GetTodayFactory $todayDateFactory)
    {
        $this->dateDiffCalculator = $dateDiffCalculator;
        $this->todayDateFactory = $todayDateFactory;
    }

    public function isUnderTheAgeOf4 (Personn $personn) {
        return $this->dateDiffCalculator->getDifferenceInYears($this->todayDateFactory->createToday(), $personn->getDateOfBirth()) < 4;
    }
}

class DateDiffCalculatorInCivilYear implements DateDiffCalculatorInYear
{
    public function getDifferencesInYears (Date $one, Date $two)
    {
        return $one - $two;
    }
}

I say "You cannot be serious about this", this is object oriented crap if you don't need to reuse DateDiffCalculatorInCivilYear.

These rules are part of your domain, so it's not a problem.

No, these rules are implementation details my domain do not care about. My domain needs a pricing system. If I was writing a pricing system, I would write an ruled based engine to get prices.

Their reuse value is next to zero? And? Are you sure? Have you got a crystal ball?

No, I'm not a seer nor an expert in pricing system. That is the exact reason why I DO NOT dare to write a system that tries to expect what the future will be like.

I do not need "as of today" those components, so I do not take the time to write them, why would I ?
I would be wasting my time doing so.

In a TDD approach, all I have to care about is getting 2 with a baby, 10 for a child and 30 for others.

"If", in the future, it shows that I "should" reuse the component, then and only then should I refactor the code and create a component.

so the fact that these classes are not reusable is not relevant for me.

Good luck writing the whole world !

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

Seems to me a better implementation:

<?php

namespace disneyland\price;

interface rule
{
    public function getPrice($personn, $price);
}

namespace disneyland\price\rules;

use disneyland\price;

class defaultPrice implements price\rule
{
    public function getPrice($personn, $price)
    {
        return 42.0;
    }
}

class under18 implements price\rule
{
    public function getPrice($personn, $price)
    {
        return ($personn->getAge() < 18 ? $price / 2 : $price);
    }
}

class hasVouncher implements price\rule
{
    public function getPrice($personn, $price)
    {
        return ($price * $personn->getVouncher());
    }
}

class isFuckingAsshole implements price\rule
{
    public function getPrice($personn, $price)
    {
        return ($personn->getNickname() === 'mageekguy' ? (float) PHP_INT_MAX : $price);
    }
}

class roulette implements price\rule
{
    public function getPrice($personn, $price)
    {
        return (rand(1, 6) === 3 ? 0.0 : $price);
    }
}

namespace disneyland;

namespace disneyland\price\rules;

class pricing
{
    private $rules = array();

    public function addRule(price\rule $rule)
    {
        $this->rules[] = $rule;

        return $this;
    }

    public function getPrice($person)
    {
        $price = 0.0;

        foreach ($this->rules as $rule)
        {
            $price = $rule->getPrice($personn, $price);
        }

        return $price;
    }
}

$price = (new pricing())
    ->addRule(new rules\defaultPrice())
    ->addRule(new rules\under18())
    ->addRule(new rules\hasVouncher())
    ->addRule(new rules\isFuckingAsshole())
    ->addRule(new rules\roulette())
    ->getPrice(new personn('mageekguy'))
;
Contributor

mageekguy commented Mar 31, 2014

Seems to me a better implementation:

<?php

namespace disneyland\price;

interface rule
{
    public function getPrice($personn, $price);
}

namespace disneyland\price\rules;

use disneyland\price;

class defaultPrice implements price\rule
{
    public function getPrice($personn, $price)
    {
        return 42.0;
    }
}

class under18 implements price\rule
{
    public function getPrice($personn, $price)
    {
        return ($personn->getAge() < 18 ? $price / 2 : $price);
    }
}

class hasVouncher implements price\rule
{
    public function getPrice($personn, $price)
    {
        return ($price * $personn->getVouncher());
    }
}

class isFuckingAsshole implements price\rule
{
    public function getPrice($personn, $price)
    {
        return ($personn->getNickname() === 'mageekguy' ? (float) PHP_INT_MAX : $price);
    }
}

class roulette implements price\rule
{
    public function getPrice($personn, $price)
    {
        return (rand(1, 6) === 3 ? 0.0 : $price);
    }
}

namespace disneyland;

namespace disneyland\price\rules;

class pricing
{
    private $rules = array();

    public function addRule(price\rule $rule)
    {
        $this->rules[] = $rule;

        return $this;
    }

    public function getPrice($person)
    {
        $price = 0.0;

        foreach ($this->rules as $rule)
        {
            $price = $rule->getPrice($personn, $price);
        }

        return $price;
    }
}

$price = (new pricing())
    ->addRule(new rules\defaultPrice())
    ->addRule(new rules\under18())
    ->addRule(new rules\hasVouncher())
    ->addRule(new rules\isFuckingAsshole())
    ->addRule(new rules\roulette())
    ->getPrice(new personn('mageekguy'))
;
@marmotz

This comment has been minimized.

Show comment
Hide comment
@marmotz

marmotz Mar 31, 2014

Contributor

Excuse me, but, in the beginning, I did not create this ticket to have a lesson to create a pricing model (even if this is actually really interesting and I learn much things because I'm a very bad developper next to you).

So, for my very first question: it's still not possible to test protected method with atoum.

Because it's still not possible, my will was to open a peaceful debate and not a open nuclear war between 2 camps.

As @guiled proposed and with the help of @jubianchi and @Hywan and the #323 issue, i'll create an extension to test protected method.
If this extension is useless for everyone, I'll be happy to be a dirty developper alone front of my screen.
If this extension is useful for many people, I'll be happy to share it and, maybe, it'll be merged in atoum, or it'll be available on atoum-extensions.org :)

Can we close this ticket or somebody absolutely want to expose this point of view ?

Contributor

marmotz commented Mar 31, 2014

Excuse me, but, in the beginning, I did not create this ticket to have a lesson to create a pricing model (even if this is actually really interesting and I learn much things because I'm a very bad developper next to you).

So, for my very first question: it's still not possible to test protected method with atoum.

Because it's still not possible, my will was to open a peaceful debate and not a open nuclear war between 2 camps.

As @guiled proposed and with the help of @jubianchi and @Hywan and the #323 issue, i'll create an extension to test protected method.
If this extension is useless for everyone, I'll be happy to be a dirty developper alone front of my screen.
If this extension is useful for many people, I'll be happy to share it and, maybe, it'll be merged in atoum, or it'll be available on atoum-extensions.org :)

Can we close this ticket or somebody absolutely want to expose this point of view ?

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

Yes, it's still not possible to test protected method with atoum because allow this enforce bad practice/design, and one of atoum's goal is to enforce good practice/design.
If someone find a valid use case for protected method test, so atoum will be updated to allow it more easily.
Currently, extension is the way for implementing that.

Contributor

mageekguy commented Mar 31, 2014

Yes, it's still not possible to test protected method with atoum because allow this enforce bad practice/design, and one of atoum's goal is to enforce good practice/design.
If someone find a valid use case for protected method test, so atoum will be updated to allow it more easily.
Currently, extension is the way for implementing that.

@marmotz

This comment has been minimized.

Show comment
Hide comment
@marmotz

marmotz Mar 31, 2014

Contributor

A valid use case for users or a valid use case for you ? :)

Contributor

marmotz commented Mar 31, 2014

A valid use case for users or a valid use case for you ? :)

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

A valid use case from OOP good practice point of view.
I'm not the author of OOP rules, so please, slap their authors if you're not agree with them, not me.
With this kind of comment, i'm not sure that you want open a peaceful debate…

Contributor

mageekguy commented Mar 31, 2014

A valid use case from OOP good practice point of view.
I'm not the author of OOP rules, so please, slap their authors if you're not agree with them, not me.
With this kind of comment, i'm not sure that you want open a peaceful debate…

@marmotz

This comment has been minimized.

Show comment
Hide comment
@marmotz

marmotz Mar 31, 2014

Contributor

The first rule of any good practices should be not to fall of the extremist side of these good practices.

OOP never ask you to code every single line of code in an useless abstract and totally confusing object. OOP respond to many problems with a lot of strong patterns. The one you presents this day is a good practice and I love this exemple. Really. Nevertheless, for a very simple case like @geraldcroes I don't see the real gain. Replace 2 lines by 6 classes and one interface is... is... I don't know how to say that... It's crazy :)

Well, I'll create an extension for my own use and I'll share it to anyone who needs to use this bad practice :)

Contributor

marmotz commented Mar 31, 2014

The first rule of any good practices should be not to fall of the extremist side of these good practices.

OOP never ask you to code every single line of code in an useless abstract and totally confusing object. OOP respond to many problems with a lot of strong patterns. The one you presents this day is a good practice and I love this exemple. Really. Nevertheless, for a very simple case like @geraldcroes I don't see the real gain. Replace 2 lines by 6 classes and one interface is... is... I don't know how to say that... It's crazy :)

Well, I'll create an extension for my own use and I'll share it to anyone who needs to use this bad practice :)

@marmotz marmotz closed this Mar 31, 2014

@mageekguy

This comment has been minimized.

Show comment
Hide comment
@mageekguy

mageekguy Mar 31, 2014

Contributor

It's crazy but it's the right way.
You don't want going in this way, fine, it's your problem.
But sadly, your way is not the atoum's way.

Contributor

mageekguy commented Mar 31, 2014

It's crazy but it's the right way.
You don't want going in this way, fine, it's your problem.
But sadly, your way is not the atoum's way.

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Mar 31, 2014

Member

It's crazy but it's the right way.

I respectfully completely disagree with your point of view. In fact, I cannot disagree more. More, I cannot understand how you can possibly believe in what you have just wrote.

Code has to be readable, self-explanatory. Your example is not, it's just an example of how you could use the "chain of responsability" pattern.

Where on hell are my business rules ? How am I suppose to even understand how the system gets its price in less than 2 seconds ? I just can't.

We really have to talk about this, because this may be object oriented, but this is way way too overkill.

Regards

Member

geraldcroes commented Mar 31, 2014

It's crazy but it's the right way.

I respectfully completely disagree with your point of view. In fact, I cannot disagree more. More, I cannot understand how you can possibly believe in what you have just wrote.

Code has to be readable, self-explanatory. Your example is not, it's just an example of how you could use the "chain of responsability" pattern.

Where on hell are my business rules ? How am I suppose to even understand how the system gets its price in less than 2 seconds ? I just can't.

We really have to talk about this, because this may be object oriented, but this is way way too overkill.

Regards

@guiled

This comment has been minimized.

Show comment
Hide comment
@guiled

guiled Mar 31, 2014

Contributor

Wow!
Reminder : "I would like to open peaceful discussion about that point"
I don't blame anyone but I think this is a passionate talk about whether or not to test protected methods.
As both answers have pros and cons, each side is right AND wrong.
If there is a meeting please, tape it and broadcast it! That will be very interesting to watch it (no irony!)

Contributor

guiled commented Mar 31, 2014

Wow!
Reminder : "I would like to open peaceful discussion about that point"
I don't blame anyone but I think this is a passionate talk about whether or not to test protected methods.
As both answers have pros and cons, each side is right AND wrong.
If there is a meeting please, tape it and broadcast it! That will be very interesting to watch it (no irony!)

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Apr 1, 2014

Member

Reminder : "I would like to open peaceful discussion about that point"

Don't worry, we don't need the reminder, this is a peaceful yet engaged discussion. Mageekguy and I know each other, we don't need to waste time in precautions while we talk ^^

Member

geraldcroes commented Apr 1, 2014

Reminder : "I would like to open peaceful discussion about that point"

Don't worry, we don't need the reminder, this is a peaceful yet engaged discussion. Mageekguy and I know each other, we don't need to waste time in precautions while we talk ^^

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Apr 1, 2014

Member

each side is right AND wrong.

And to be a little extreme, no, and no again, there is nothing wrong in implementing business rules as they are in a single class if the class has this one and specific responsibility.

It is the way to go.

Don't get me wrong, you have to write reusable code, but reusable just means "I do what I have to do, no more, no less".

I can bet a large amount of beers that my example can be patched by anyone in a development team in less than five minutes when it comes to answer "If the order contains 3 products or more, with 2 childrens, make one children free". Using the rule system, so called "generic and reusable", you'll have to take your time watch them struggle their way through.

YAGNI (You ain't gonna need it) is true, even more when you do not actually need it.

And if you really want to be convinced, imagine now the associated unit test.

Unit test associated with my example would be "First, give a mock order with a baby, 2 euros, ok. Second, give a mock order with a child, 4 euros, ok, ..."

With the rule engine, the business disappears, "first, create a rule engine with mocked rules, give it a mock order that match one of the mocked rule, check if it was called"... what ? really ? so... how many euros ? what am I expected ? Still don't know.

Member

geraldcroes commented Apr 1, 2014

each side is right AND wrong.

And to be a little extreme, no, and no again, there is nothing wrong in implementing business rules as they are in a single class if the class has this one and specific responsibility.

It is the way to go.

Don't get me wrong, you have to write reusable code, but reusable just means "I do what I have to do, no more, no less".

I can bet a large amount of beers that my example can be patched by anyone in a development team in less than five minutes when it comes to answer "If the order contains 3 products or more, with 2 childrens, make one children free". Using the rule system, so called "generic and reusable", you'll have to take your time watch them struggle their way through.

YAGNI (You ain't gonna need it) is true, even more when you do not actually need it.

And if you really want to be convinced, imagine now the associated unit test.

Unit test associated with my example would be "First, give a mock order with a baby, 2 euros, ok. Second, give a mock order with a child, 4 euros, ok, ..."

With the rule engine, the business disappears, "first, create a rule engine with mocked rules, give it a mock order that match one of the mocked rule, check if it was called"... what ? really ? so... how many euros ? what am I expected ? Still don't know.

@marmotz

This comment has been minimized.

Show comment
Hide comment
@marmotz

marmotz Apr 1, 2014

Contributor

If you allow, I closed this ticket because the discussion, although very interesting, totally away from the initial debate.

I did not need a lesson on OOP just because neither @geraldcroes nor I could not find an use case that has convinced you.
Many of us feel the need to test occasionally protected methods and the debate was whether it could be a good idea to implement it in one way or another and find the better one.

The discussion was initially cordial and constructive. Then it deflected in a totally unnecessary and irrelevant lesson.

Or this discussion can focus on the original question and I reopen it, or you keep your childishness about OOP elsewhere.

Nobody needs to read this and be sure that it does not grow anyone, especially the community of atoum.

Contributor

marmotz commented Apr 1, 2014

If you allow, I closed this ticket because the discussion, although very interesting, totally away from the initial debate.

I did not need a lesson on OOP just because neither @geraldcroes nor I could not find an use case that has convinced you.
Many of us feel the need to test occasionally protected methods and the debate was whether it could be a good idea to implement it in one way or another and find the better one.

The discussion was initially cordial and constructive. Then it deflected in a totally unnecessary and irrelevant lesson.

Or this discussion can focus on the original question and I reopen it, or you keep your childishness about OOP elsewhere.

Nobody needs to read this and be sure that it does not grow anyone, especially the community of atoum.

@geraldcroes

This comment has been minimized.

Show comment
Hide comment
@geraldcroes

geraldcroes Apr 1, 2014

Member

I do not feel that we're taking part in any kind of childish game, we're trying to find a common ground to assess wether or not protected methods should be tested.

To assess this, we are struggling to find a use case, considered valid.

To me, the discussion is still cordial and constructive. We both give examples, we both try to point out relevant details and provide insights.

Nobody needs to read this and be sure that it does not grow anyone, especially the community of atoum.

I need to read the answers, it is interesting to me and makes me better : I have to take my point of view down in such a way everyone can understand it whilst considering it a valid use case (as I truly believe it is, and I've been advocating it for years now).

The community of atoum is very welcome to the discussion, even more if they do not agree with neither of us. We need opinions, we need convictions, we need debates.

Member

geraldcroes commented Apr 1, 2014

I do not feel that we're taking part in any kind of childish game, we're trying to find a common ground to assess wether or not protected methods should be tested.

To assess this, we are struggling to find a use case, considered valid.

To me, the discussion is still cordial and constructive. We both give examples, we both try to point out relevant details and provide insights.

Nobody needs to read this and be sure that it does not grow anyone, especially the community of atoum.

I need to read the answers, it is interesting to me and makes me better : I have to take my point of view down in such a way everyone can understand it whilst considering it a valid use case (as I truly believe it is, and I've been advocating it for years now).

The community of atoum is very welcome to the discussion, even more if they do not agree with neither of us. We need opinions, we need convictions, we need debates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment