Skip to content

Feature/all katas #19

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 48 commits into
base: develop
Choose a base branch
from
Open

Conversation

jpaitken
Copy link

Mine

*/
public final class BankOCR {

private static Map<Integer, String> mapNum = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be final

final int finalFactor = 10;
final int aux = 48;
for (int i = 0; i < finalNumber; i++) {
if (i % 2 != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using an if else
Consider using a ternary condition.

public int evaporator(final double content, final int evap, final int threshold) {
int days = 0;
double waste1;
double limit1 = content * threshold / 100.00;
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 variable?

double limit1 = content * threshold / 100.00;
double conte = content;
while (conte >= limit1) {
waste1 = conte * evap / 100.00;
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 variable?

* @param sentence is the word or words
* @return the word spined.
*/
public static String spinWords(final String sentence) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of code. :(
Try to refactor it

public void testNewRealeasenMovieAmount() {

// given:
final NewRelease newRelease = new NewRelease("Logan");
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be

final Movie newRelease = new NewRelease("Logan");

Copy link
Contributor

Choose a reason for hiding this comment

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

Apply in all your unit tests

coding.iml Outdated
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
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 unnecessary file

@codecov-io
Copy link

codecov-io commented Jul 1, 2017

Codecov Report

Merging #19 into develop will increase coverage by 0.3%.
The diff coverage is 95.67%.

Impacted file tree graph

@@             Coverage Diff              @@
##             develop      #19     +/-   ##
============================================
+ Coverage      94.78%   95.09%   +0.3%     
- Complexity       138      210     +72     
============================================
  Files             26       39     +13     
  Lines            307      469    +162     
  Branches          50       73     +23     
============================================
+ Hits             291      446    +155     
- Misses             5        6      +1     
- Partials          11       17      +6
Impacted Files Coverage Δ Complexity Δ
...g/fundacionjala/coding/juan/test/ArrayAverage.java 100% <100%> (ø) 6 <6> (?)
...fundacionjala/coding/juan/spinWords/SpinWords.java 100% <100%> (ø) 5 <5> (?)
...coding/juan/multiplesof3and5/MultiplesOf3And5.java 100% <100%> (ø) 5 <5> (?)
...oding/juan/highestandlowest/HightestAndLowest.java 100% <100%> (ø) 3 <3> (?)
...org/fundacionjala/coding/juan/movies/Customer.java 100% <100%> (ø) 8 <8> (?)
...va/org/fundacionjala/coding/juan/movies/Movie.java 100% <100%> (ø) 2 <2> (?)
...org/fundacionjala/coding/juan/movies/Children.java 100% <100%> (ø) 4 <4> (?)
.../org/fundacionjala/coding/juan/movies/Regular.java 100% <100%> (ø) 4 <4> (?)
.../org/fundacionjala/coding/juan/gas/Evaporator.java 100% <100%> (ø) 3 <3> (?)
...a/org/fundacionjala/coding/juan/movies/Rental.java 100% <100%> (ø) 5 <5> (?)
... and 16 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...8209bc7. Read the comment docs.

Coverage 90%
if (i % 2 != 0) {
sum += ((eanCode.charAt(i) - aux) * multiply);
} else {
sum += (eanCode.charAt(i) - aux);
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 use a ternary condition here

* @param sentence is the word or words
* @return the word spined.
*/
public static String spinWords(final String sentence) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please refactor this code

* Created by Administrator on 6/28/2017.
*/
public class EvaporatorTest {
private Evaporator evaporator = new Evaporator();
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bad practice

* Created by Administrator on 6/28/2017.
*/
public class HightesAndLowestTest {
private HightesAndLowest hightesAndLowest = new HightesAndLowest();
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a bad practice

/**
* Created by Juan Pablo on 26/03/2017.
*/
public final class BankOCR {
Copy link
Contributor

Choose a reason for hiding this comment

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

This solution is not completed


for (Rental rental : rentalsCustomer) {

result += String.format("%s ", rental.getMovie().getTitle());
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 sentence is the word or words
* @return the word reversed.
*/
public static String spinWords(final String sentence) {
Copy link
Contributor

@carledriss carledriss Jul 7, 2017

Choose a reason for hiding this comment

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

you can still improve this method.

String account = "021453789";

// when:
String expectedResult = "ERR";
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[] accountArray = BankOCR.parseScannedFigures(figureScanned);

// when:
String actualResult = BankOCR.accountRepresentation(accountArray);
Copy link
Contributor

Choose a reason for hiding this comment

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

final

* Testing an invalid account.
*/
@Test
public void verifyValidateAccountNumbersFalse() {
Copy link
Contributor

@carledriss carledriss Jul 7, 2017

Choose a reason for hiding this comment

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

@AT-03/at-03
all your unit tests should start with the prefix test not verify

* Initialize variables.
*/
@Before
public void initialize() {
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
Consider using setUp instead of initialize

* Created by Administrator on 6/28/2017.
*/
public class MultiplesOf3And5Test {
private MultiplesOf3And5 kata = new MultiplesOf3And5();
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?

int checkSum = 0;
int digit = 0;
for (int i = 0; i < code.length; i++) {
digit = (code[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

remove unnecessary parenthesis

int digit = 0;
for (int i = 0; i < code.length; i++) {
digit = (code[i]);
checkSum += ((i + 1) % 2 == 0) ? digit * 3 : digit;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove unnecessary parenthesis

double cont = content;
double limit1 = content * threshold / 100.00;
while (cont >= limit1) {
cont = cont - cont * evap / 100.00;
Copy link
Contributor

Choose a reason for hiding this comment

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

please refactor this line of code

public NewRelease(final String title) {
super(title);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

{@inherentDoc}


@Override
public int calculateFrequentRenterPoints(final int daysRented) {
return 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

use a constant

" _ _ _ _ _ _ _ "
+ " ||_||_||_||_| _";
// when:
final Boolean actualResult = BankOCR.validateAccountNumbers(figureScanned);
Copy link
Contributor

Choose a reason for hiding this comment

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

why are using the Boolean class?
you should use the primitive boolean

* Created by Administrator on 7/2/2017.
*/
public class RentalTest {
private Regular regular;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is wrong here?

* @param account account.
* @return true or false.
*/
static boolean isLegible(final String account) {
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