Skip to content

Solution for Movie Recommender 2019b #63

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: master
Choose a base branch
from

Conversation

alonsosfera
Copy link

@alonsosfera alonsosfera commented Oct 5, 2019

Solution for Movie Recommender using Mahout in Java.
Alonso Gutierrez. Academy 2019b

private Hashtable<String, Integer> HashProduct = new Hashtable<String, Integer>();
private Hashtable<Integer, String> InvertedHashProduct = new Hashtable<Integer, String>();
private Hashtable<String, Integer> HashUser = new Hashtable<String, Integer>();
private int users =0, products =0, reviews = 0;

Choose a reason for hiding this comment

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

always format your code =0 vs = 0


public class MovieRecommender {
private String file;
private Hashtable<String, Integer> HashProduct = new Hashtable<String, Integer>();

Choose a reason for hiding this comment

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

why did you choose Hashtable vs HashMap?

Copy link
Author

Choose a reason for hiding this comment

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

I was trying different ways of handling bigs amounts of data to see which would give me faster results for the tests, and that's the way I found when working on the solution and thought was the one that would do better, but after doing some research, I understand now that there is nothing in the hashtable that can't be done using hashmap...

Choose a reason for hiding this comment

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

cool, just wanted you to investigate the difference, which is synchronization please research on that

Copy link
Author

Choose a reason for hiding this comment

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

I actually was reading about that. I also read that Hashtables would be used for thread-safe applications, but now we can use Collections.synchronizedMap() or ConcurrentHash instead.

}
}
}
br.close();

Choose a reason for hiding this comment

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

I don't like this is not in a finally block. have you heard about try-catch-with-resources? please investigate about it.


int userValue = HashUser.get(user);

List RecommendedProducts = new ArrayList<String>();

Choose a reason for hiding this comment

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

ArrayList has a constructor that receives an initialCapacity why not using it based in recommendations size?

Can you answer what benefit would it bring implementing this suggestion?

Copy link
Author

Choose a reason for hiding this comment

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

Giving an initial capacity to the ArrayList will reduce the use of memory keeping the size of the array only at whatever we are going to use

// http://snap.stanford.edu/data/web-Movies.html
MovieRecommender recommender = new MovieRecommender("/path/to/movies.txt.gz");
MovieRecommender recommender = new MovieRecommender("/Users/alonso/Documents/big-data-exercises/movies.txt.gz");

Choose a reason for hiding this comment

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

hard coded paths doesn't looks good at all

Copy link
Author

Choose a reason for hiding this comment

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

Fixed...

if (line.length() >= 0) {
sp = line.split(" ");
key = sp[0];
if (key.equals("product/productId:")) {

Choose a reason for hiding this comment

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

key.equals("product/productId:") can produce a null pointer exception,
"product/productId:".equals(key) can't, something to have in mind.

getData();
}

public String getData() throws IOException {

Choose a reason for hiding this comment

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

why public?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed


while((line = br.readLine()) != null) {
if (line.length() >= 0) {
sp = line.split(" ");

Choose a reason for hiding this comment

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

what if there are no blank spaces in the line being split?

Copy link
Author

Choose a reason for hiding this comment

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

I would not be able to get the correct values or keys... Fixed

}
br.close();
bw.close();
return null;

Choose a reason for hiding this comment

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

returning always null will be helpful somehow?

Copy link
Author

Choose a reason for hiding this comment

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

Changed getData() to private void instead of public String

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