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

All entities should have only int ID, String should be built in toString method #65

Open
F-I-D-O opened this issue Oct 18, 2018 · 3 comments
Labels
conceptual postponed Issues that possibly needs to be resolved in the future if some prerequisite appears

Comments

@F-I-D-O
Copy link
Member

F-I-D-O commented Oct 18, 2018

No description provided.

@marekcuchy
Copy link
Collaborator

I think that String IDs are better for this purpose. It allows us to say something about e.g. the type of the entity directly in the ID making e.g. debugging easier and also logs from the simulation are easier to read.

Is there any reason why String should be replaced?

@F-I-D-O
Copy link
Member Author

F-I-D-O commented Jul 28, 2020

If the ID type is not bound to int, it leads to a mess in logs, where messages like "Request request 345" appears. I think that if you want to say something about the type, the toString method is appropriate for that.

@marekcuchy marekcuchy added postponed Issues that possibly needs to be resolved in the future if some prerequisite appears and removed awaiting discussion labels Aug 3, 2020
@marekcuchy
Copy link
Collaborator

marekcuchy commented Aug 3, 2020

After a short discussion, this issue is postponed until integer IDs would bring significant performance benefit (which it does not now).

String IDs can be self-descriptive to provide better human readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptual postponed Issues that possibly needs to be resolved in the future if some prerequisite appears
Projects
None yet
Development

No branches or pull requests

2 participants