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

CommoditySet only supports three commodities #33

Open
ewoudwerkman opened this issue Feb 5, 2015 · 2 comments
Open

CommoditySet only supports three commodities #33

ewoudwerkman opened this issue Feb 5, 2015 · 2 comments

Comments

@ewoudwerkman
Copy link

For each commodity a boolean is used.

If more commodities are specified, the CommoditySet class has to be extendend for each commodity added. If you don't know about this class, things may go wrong. Therefore this class is not really generic.

This is not future proof, preferably the iterator returns the commodity itself, instead of the booleans

@marcdejonge
Copy link
Contributor

The CommoditySet is really a Set, in the sense that you can ask it for which Commodity objects are in it. Since there are only 3 possible Commodities right now (it is an enumeration, somewhat), it seemed simpler to implement it with 3 boolean value. On the outside it acts perfectly according to the Set interface.

@wilcowijbrandi
Copy link
Member

But the EFI classes really want to see a CommoditySet, not a Set containing commodies. I think the proper way would be to demand a Set<Commodity> in the EFI classes, and in practice always use the CommoditySet since it is more efficient.

@wilcowijbrandi wilcowijbrandi reopened this May 4, 2015
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

No branches or pull requests

3 participants