prmr / Creco

Recommendation System for Consumer Products
Apache License 2.0
6 stars 2 forks source link

"N/A" Typed Values #50

Closed asutcl closed 10 years ago

asutcl commented 10 years ago

"N/A" or "NA" is sometimes used interchangeably in the code to specifiy that no value was available for this attribute. This can lead to errors.

TypedValue should also be cleaned of dead code.

Add equals and hashcode.

prmr commented 10 years ago

ProductID, value, AttributeID 83497,NA,1155 83496,NA,1155 83495,NA,1155 207596,N/A,3373 207598,N/A,3373 159097,NA,3373 159092,NA,3373 191702,NA,3371 191146,N/A,4953

prmr commented 10 years ago

Frequency in the test database: Ratings: 162 Specs: 2348

prmr commented 10 years ago

TypedValue now deals with this properly. asutcl, are there any changes to do in the logic component related to this issue?

asutcl commented 10 years ago

Sorry I didn't give enough specifications when I wrote this issue. I had added "N/A" for scores sometimes if they were not properly formatted. When I was debugging code the other day I noticed that in the SearchController and the Filtering Test this was also done. I think the specific value we use for improperly formatted data should be handled by TypedValue. Is it possible to make TOKEN_NA public static final in TypedValue and then all code should get that string instead of creating it locally in their code?

asutcl commented 10 years ago

If not what is the best way to ensure everyone handles "NA" the same way?

prmr commented 10 years ago

I think it should be to create NA TypedValues: new TypedValue("NA") and add a check for that: value.isNA() which would be much cleaner. We could even make the default constructor of TypedValue create an NA value. Thoughts?

asutcl commented 10 years ago

Yes, that would work.

prmr commented 10 years ago

What's the point of nominal value? Can I just remove this and replace it with getValue()? It's only used for strings anyways.

prmr commented 10 years ago

I'm also going to remove INTEGER and DOUBLE and replace with NUMERIC

prmr commented 10 years ago

asutcl: I've integrated a much better version of the TypedValue in Branch Issue0050 but it seems to break something in AttributeExtractor: the AttributeMean field gets null. Can you have a look to see if there's a quick fix?

prmr commented 10 years ago

Actually the problem was in SearchController, not AttributeExtractor. I'm fixing it now.

prmr commented 10 years ago

New TypeValue API available in the master branch.