Skip to content

all katas #28

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

caberoDaniel
Copy link

all katas of the coding.

@codecov-io
Copy link

codecov-io commented Jul 6, 2017

Codecov Report

Merging #28 into develop will increase coverage by 0.22%.
The diff coverage is 95.45%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop      #28      +/-   ##
=============================================
+ Coverage      94.78%   95.01%   +0.22%     
- Complexity       138      210      +72     
=============================================
  Files             26       40      +14     
  Lines            307      461     +154     
  Branches          50       70      +20     
=============================================
+ Hits             291      438     +147     
- Misses             5        9       +4     
- Partials          11       14       +3
Impacted Files Coverage Δ Complexity Δ
...acionjala/coding/danielcabero/movies/Children.java 100% <100%> (ø) 4 <4> (?)
...ionjala/coding/danielcabero/movies/NewRelease.java 100% <100%> (ø) 4 <4> (?)
...elcabero/sorttheinnercontent/SortInnerContent.java 100% <100%> (ø) 6 <6> (?)
...anielcabero/highestandlowest/HighestAndLowest.java 100% <100%> (ø) 2 <2> (?)
...ala/coding/danielcabero/evaporator/Evaporator.java 100% <100%> (ø) 3 <3> (?)
...anielcabero/multiplesof3and5/MultiplesOf3And5.java 100% <100%> (ø) 5 <5> (?)
...undacionjala/coding/danielcabero/movies/Movie.java 100% <100%> (ø) 2 <2> (?)
...ndacionjala/coding/danielcabero/movies/Rental.java 100% <100%> (ø) 5 <5> (?)
...anielcabero/averageofnumbers/AverageOfNumbers.java 100% <100%> (ø) 5 <5> (?)
...acionjala/coding/danielcabero/movies/Customer.java 100% <100%> (ø) 7 <7> (?)
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 112c303...2f06f4d. Read the comment docs.

* @param numberAccount image of type String.
* @return <code>true</code> \\ <code>false</code>
*/
private static boolean size(final String numberAccount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

refactor this method
it can be implemented in a single line of code

* @param numberAccount image of type String.
* @return <code>true</code> \\ <code>false</code>
*/
private static boolean size(final String numberAccount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why this method is called size ?

* @param scannedAccount of String type.
* @return true.
*/
private static boolean sizeImageIsValid(final String scannedAccount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this method?

* @return true if the checksum is ok.
*/

public static boolean validate(final String eAN) {
Copy link
Contributor

Choose a reason for hiding this comment

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

refactor this code
avoid the else condition

* @param stringNumber number
* @return true if stringNumber length is exactly 13.
*/
public static boolean checkCorrectLength(final String stringNumber) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this method?

/**
* Created by administrator on 3/10/2017.
*/
public final class Main {
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this class since you will add unit tests

};

// when:
String actualResult = BankOcr.numberAccountPresentImage(scannedImage);
Copy link
Contributor

Choose a reason for hiding this comment

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

final

String actualResult = BankOcr.numberAccountPresentImage(scannedImage);

// then:
String expectedResult = "0123456789";
Copy link
Contributor

Choose a reason for hiding this comment

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

final

boolean actualResult = BankOcr.validateNumberAccount(incorrectAccount);

// then:
assertTrue(!actualResult);
Copy link
Contributor

Choose a reason for hiding this comment

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

assertFalse

final boolean actualResult = EANValidator.checkCorrectLength(eanStringNumber);

// then:
assertTrue(!actualResult);
Copy link
Contributor

Choose a reason for hiding this comment

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

assertFalse

final boolean actualResult = EANValidator.validate(eanStringNumber);

// then:
assertTrue(!actualResult);
Copy link
Contributor

Choose a reason for hiding this comment

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

assertFalse


// then
final double expectedResult = 1.5;
assertEquals(0, expectedResult, actualResult);
Copy link
Contributor

Choose a reason for hiding this comment

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

first argument should be expectedResult and the second argument should be actualResult

@Test
public void testRentalEmpty() {
// given:
Customer customer = new Customer("Test");
Copy link
Contributor

Choose a reason for hiding this comment

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

you are repeating this code in all your test
Consider put it in a @before method

/**
* Created by Administrator on 7/5/2017.
*/
public class MainTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this test class?

/**
* Created by Administrator on 7/5/2017.
*/
public class MovieTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

@AT-03/at-03
Do we need this unit tests?

@Test
public void testCalculateAmountWhenTheRentedDaysIsLessThanThree() {
// given:
NewRelease newReleaseInstance = new NewRelease("Test");
Copy link
Contributor

Choose a reason for hiding this comment

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

@AT-03/at-03
what is wrong here?

*/
public class RentalTest {

private Movie movie = new Movie("test") {
Copy link
Contributor

Choose a reason for hiding this comment

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

@AT-03/at-03
what is wrong here?

* @param numberAccount image of type String.
* @return <code>true</code> \\ <code>false</code>
*/
private static boolean isCorrectLength(final String numberAccount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can still refactor this

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.

3 participants