Skip to content

Conversation

carousel
Copy link
Contributor

@carousel carousel commented Apr 29, 2018

Hi folks;

I have enhanced delegation pattern example by sending $this object as context (TeamLead) to delegate (Junior)

Cheers

Copy link
Contributor

@piotrgradzinski piotrgradzinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I think the approach is ok. Needs some formatting changes.
@domnikl - would you like to merge?

@@ -8,4 +8,11 @@ public function writeBadCode(): string
{
return 'Some junior developer generated code...';
}
/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are changing the file please work on proper formatting. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, tnx. Comments was added just to explain reason for change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carousel - I mean new line before comments of course :)

Copy link
Contributor

@piotrgradzinski piotrgradzinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. @domnikl ?

@@ -21,4 +21,17 @@ public function writeCode(): string
{
return $this->junior->writeBadCode();
}
public function writeBadCode(): string

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs a space between the methods definitions

/**
* @test
*/
public function teamLeadCanBlameJuniorForBadCode()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use the test in method name convention here instead of using @test: testTeamLeadCanBlameJuniorForBadCode

@domnikl
Copy link
Contributor

domnikl commented May 13, 2018

@carousel could you please fix the issues in the PR mentioned by @malukenho and me? Then it is ready to get merged.

@carousel
Copy link
Contributor Author

carousel commented May 13, 2018

sure, working on it
pushed

@domnikl domnikl merged commit fb42e71 into DesignPatternsPHP:master May 17, 2018
@domnikl
Copy link
Contributor

domnikl commented May 17, 2018

@carousel merged, thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants