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

Environment is highly coupled with childs of EnvironmentObject #16

Open
MarkLai0317 opened this issue Jun 21, 2024 · 7 comments
Open

Environment is highly coupled with childs of EnvironmentObject #16

MarkLai0317 opened this issue Jun 21, 2024 · 7 comments
Assignees
Labels
discussion Discuss about some design or implmentation refactor About to refactor some part of code

Comments

@MarkLai0317
Copy link
Collaborator

potential recurrent problem: the Environment is coupled with Food and Organism, which will limit the scalability if there are new child of EnvironmentObject or Food, Organism . That is, we need to add new getFunction or new map in Environment.

solution: I suggest we can apply visitor pattern so that we can add different getter without hard coding FoodAllFoods, getAllOrganism . That is, we can define different getter and use it as parameter in the one and only getEnvironmentObject(getterFilter).

Expected result: Environment only depend on EnvironmentObject

@YJack0000
Copy link
Owner

Actually, when I was working on this project, ChatGPT suggested using the Visitor pattern many times. However, I’m not really familiar with it.

Is that possible to use UML to show what the changes would look like?
Especially about the decouple of Environment and Organism.

@MarkLai0317
Copy link
Collaborator Author

MarkLai0317 commented Jun 21, 2024

Here is the uml to demonstrate the result of refactoring. the add and remove is changed to accept EnvironmentObject instead of Food and Organism.

The EnvironmentObjectGetter is what others call Visitor and getObject is what's called visit in visitor pattern.

Visitor pattern can do a lot more than just get. But I currently only use it on Getter. Other usage can be discussed in the future

For the deadOrganism. I suggest we can add variable to Organism to indicate dead. Or is there a reason we need to store them in a different vector ?

@MarkLai0317 MarkLai0317 added discussion Discuss about some design or implmentation refactor About to refactor some part of code labels Jun 21, 2024
@YJack0000
Copy link
Owner

YJack0000 commented Jun 21, 2024

This approach decouples Environment from different EnvironmentObject implementations, which is beneficial for adding new features.

However, two points should be considered:

  1. Exposing features to Python users: Directly exposing getObject and Getters might not be intuitive for scripting.
  2. Customizability: While refactoring increases customizability, it raises a concern related to feature How to make the interaction and reaction between EnvironmentObject be customizable #11. If users can customize different EnvironmentObjects, how will they manage their interactions?

@YJack0000
Copy link
Owner

The deadOrganism vector is important for the simulation, and I included it for convenience. However, adding a status attribute to organisms might be a good alternative since iterating through all organisms to find the dead ones is not very intensive.

Maybe we could add another discussion or refactor issue to talk about this.

@MarkLai0317
Copy link
Collaborator Author

MarkLai0317 commented Jul 7, 2024 via email

@YJack0000
Copy link
Owner

Can you provide some syntax for me to better understand what it will look like?

@MarkLai0317 MarkLai0317 linked a pull request Jul 15, 2024 that will close this issue
@MarkLai0317
Copy link
Collaborator Author

I have redesign the getObject method that accept a custom filter which accept a EnvironmentObject pointer and return a vector that satisfy the custom filter.

This will not expose too much things to users. and it's quite common for python scripting like below.

my_list = [12, 65, 54, 39, 102, 339, 221, 50, 70] 
# use anonymous function to filter and comparing  
# if divisible or not 
result = list(filter(lambda x: (x % 13 == 0), my_list))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discuss about some design or implmentation refactor About to refactor some part of code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants