Skip to content

Cynthia refactor video#56

Open
CynthiaTerrazas wants to merge 12 commits intodevelopfrom
CynthiaVideo
Open

Cynthia refactor video#56
CynthiaTerrazas wants to merge 12 commits intodevelopfrom
CynthiaVideo

Conversation

@CynthiaTerrazas
Copy link

No description provided.

*/
public class Customer {
private String customerName;
private Vector movieRentals = new Vector();
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 a List instead of a Vector

@carledriss
Copy link
Contributor

carledriss commented Jun 9, 2017

Consider using inheritance and polymorphism

/**
* @return total cost for all movies rented.
*/
public String totalCostRented() {
Copy link
Contributor

@carledriss carledriss Jun 13, 2017

Choose a reason for hiding this comment

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

This method is still calculating the totalCost and it is printing.
Please split it.

/**
* @return return the bonus for all movies rented.
*/
public String totalBonusFrequencyRented() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is still calculating the totalFrequentRenterPoints and it is printing.
Please split it.

public static final int REGULAR = 0;
public static final int NEW_RELEASE = 1;
private String titleMovie;
private int priceCodeMovie;
Copy link
Contributor

Choose a reason for hiding this comment

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

Now you need to remove the CHILDRENS, REGULAR, NEW_RELEASE constants and the priceCodeMovie attribute
and take advantage of inheritance. :)

private String customerName;
private List<Rental> movieRentals;
private double totalAmount = 0;
private int frequentRenterPoints;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these attributes are no longer necessary because you have methods that are returned the totalAmount and totalFrequentRenterPoints.

* @param title movie.
* @param priceCode movie.
*/
Movie(String title, int priceCode) {
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 still need the priceCode?

if (rental.getDaysRented() > 1) {
return 1;
}
return 0;
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 method in a single line.

@carledriss
Copy link
Contributor

@CynthiaTerrazas
please apply the code review observations

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