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

Movie Recommender Zeus #37

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Zeusjonass
Copy link

To run the test it's necessary to download and unzip the file 'movies.txt' from: http://snap.stanford.edu/data/web-Movies.html


public class MovieRecommender {

private int totalReviews = 0;

Choose a reason for hiding this comment

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

redundant initialization, in Java the primitives are initialized by default in this case to 0

private BidiMap<String, Integer> products = new DualHashBidiMap<>();

public MovieRecommender(String moviesPath) {
try {
Copy link

@linolarios linolarios Oct 26, 2021

Choose a reason for hiding this comment

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

You can have some of the logic in the constructor but not the entire functionality, also a code full of conditionals is a bad signal, try to clean up the code with polymorphism or at least use a switch statement

String userId = null;

while((line = br.readLine()) != null){
if (line.startsWith("review/score")) {

Choose a reason for hiding this comment

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

use constants to avoid magic numbers and repeated string

FileReader fr = new FileReader(file);
BufferedReader br = new BufferedReader(fr);

File csvFile = new File("movies.csv");

Choose a reason for hiding this comment

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

read these values from a property file to make your code more flexible and maintainable

@@ -15,7 +15,7 @@
public void testDataInfo() throws IOException, TasteException {

Choose a reason for hiding this comment

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

Please use a meaningful name for the tests, something like the intention of your test, what you are trying to show, and provide more context to the assertions; the test not only proves that your code works fine, you are providing 'live' documentation.

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.

2 participants