Skip to content
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

[18] - implements koans solutions #19

Merged

Conversation

IvanSolaDelgado
Copy link

@IvanSolaDelgado IvanSolaDelgado commented Jul 12, 2023

Issue

Solution

  • I have created the koansSolutions folder and in it I have implemented all the solutions of the koans that we have

Notes

  • I have added to this PR some changes suggested in PR [16] - Adjust test and comments #17 by @AsierAlba10 to avoid having to implement it twice, these changes are improvements in the naming of some tests, creating the koansResources directory (where the classes and exceptions go) and extracting one AssertKoans class outside of the file.

@IvanSolaDelgado IvanSolaDelgado self-assigned this Jul 12, 2023
@IvanSolaDelgado IvanSolaDelgado changed the base branch from master to 16-adjust-test-and-comments July 12, 2023 12:00
@IvanSolaDelgado IvanSolaDelgado changed the title 18 implements koans solutions [18] - implements koans solutions Jul 13, 2023
Copy link
Collaborator

@AsierAlba10 AsierAlba10 left a comment

Choose a reason for hiding this comment

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

Ok to merge, be careful with the structure of the name of the PR the first one is always capitalized


// Create a ForEach Loop to pass the test
$result = '';
foreach ($age as $key => $value){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe you can refactor the names of the forEach
foreach ($age as $key => $value){ ->
foreach ($people as $name => $age){

// Create a ForEach Loop to pass the test
$result = '';
foreach ($age as $key => $value){
$result .= "Key= $key, Value= $value ; ";
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you think if we change the answer to make it look better?
"Key = $key, Value = $value ; "

}

// Extract Joe´s Age
$this->assertEquals($result, "Key= Joe, Value= 49 ; Key= Mike, Value= 27 ; Key= Charles, Value= 32 ; ");
Copy link
Collaborator

Choose a reason for hiding this comment

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

This changes

* @test
* @testdox Some ways of asserting equality are better than others
*/
public function checkHowMuchIsOnePlusOneUsingVariablesAndAssertEquals()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test and checkHowMuchIsOnePlusOne(2 above) they are not repetitive

@IvanSolaDelgado IvanSolaDelgado merged commit c51c24b into 16-adjust-test-and-comments Jul 17, 2023
@IvanSolaDelgado IvanSolaDelgado deleted the 18-implements-koans-solutions branch July 17, 2023 06:21
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.

Koans solutions
2 participants