Skip to content

Update propagation module architecture to accept different P2P models#969

Open
mglesser wants to merge 24 commits into
Universite-Gustave-Eiffel:mainfrom
mglesser:p2p-propagation-model-archi
Open

Update propagation module architecture to accept different P2P models#969
mglesser wants to merge 24 commits into
Universite-Gustave-Eiffel:mainfrom
mglesser:p2p-propagation-model-archi

Conversation

@mglesser

Copy link
Copy Markdown
Contributor

Add an interface and a factory class for propagation models.

mglesser added 23 commits May 21, 2026 18:24
@mglesser mglesser marked this pull request as ready for review June 15, 2026 12:11
@mglesser mglesser requested a review from a team June 15, 2026 12:11
* @param modelName Name of the propagation model
* @return propagation model object
*/
public static PropagationModel create(String modelName){

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the factory method pattern:

Image

The goal of a factory is to be able to add new propagation model without having to change the source code of noisemodelling-propagation

So in the main NoiseMapByReceiverMaker, you provide an implementation of the PropagationModelFactory (PropagationModelFactory being an Interface or abstract class)

NoiseMapByReceiverMaker have a default implementation of PropagationModelFactory that is the Cnossos one

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can look at
IComputeRaysOutFactory
TableLoader

That provide such pattern (can be replaced by another implementation)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've modified the code to implement a proper factory method for the propagation models.

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

Successfully merging this pull request may close these issues.

2 participants