search

prmr / Creco

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

Clean up Data API #22

Closed prmr closed 10 years ago

prmr commented 10 years ago

For all files of data and data.stubs:

Branch: Issue0022

enewe101 commented 10 years ago

Re merging CategoryList and ProductList into CRData -- from the standpoint of accessing the API it definitely makes sense to get rid of the need to first get the List objects. The List classes do have some logic used when building up the data representation after loading -- things like CategoryList.findEquivalenceClasses(). So I guess that would just all move into CRData? Yeah, I don't see why not.

BTW, glad we have the loading service now -- that was clearly needed!

prmr commented 10 years ago

Any reason why the franchises and all their children can't be indexed on the fly, as they are added in? I would get rid of the franchises field in CategoryList and just keep everything in a hashmap from the get-go, indexing as the new objects come in.

enewe101 commented 10 years ago

Yup, they could be indexed on the fly.

But I think the franchises field is needed, unless there is a better way to provide an iterable of franchises? My motivation for having an iterable over franchises is that it provides a convenient entry point for recursive algorithms running over the category tree.

prmr commented 10 years ago

The API should be stable now. The only thing left is I'd like to move some of the complex computations out of Category, but this is less critical. If you have time, please feel free to start writing unit tests into the the Issue0022 branch.

enewe101 commented 10 years ago

Ok, I think I see the intended division.

The idea is to move the computations out of category, and into some kind of category-processor, right? Then categories only handle the business of data representation, while the processor operates on categories, either to yield computed results or restructure them -- is that it?

On Sun, Mar 2, 2014 at 12:25 PM, prmr notifications@github.com wrote:

The API should be stable now. The only thing left is I'd like to move some of the complex computations out of Category, but this is less critical. If you have time, please feel free to start writing unit tests into the the Issue0022 branch.

Reply to this email directly or view it on GitHubhttps://github.com/prmr/Creco/issues/22#issuecomment-36459886 .

prmr commented 10 years ago

Possibly, but this might not make it into this issue. Ultimately I just want to reduce the public interface of the Category class. I'm about to push a change where I've just reduced the accessibility to package-private of all methods that should not be used by clients, so that I can release the API to the master branch.

prmr commented 10 years ago

What's the role of the Category.aCount field?

prmr commented 10 years ago

Merged into master