Simplify config #40
Simplify config #40
Conversation
|
I like this simplification. I actually had in mind replacing the current config file reader for the typesafe one that you proposed. I also like the creation of different classes for related parameters. Regarding the passing of these classes as arguments to the various agents constructors, I generally like it, but I seem to remember there were at least a couple of cases where the current config object had to be declared as static because some parameter needed to be used without any instantiation... Oh, and bear in mind that this branch is (technically) now one commit behind the master... one can probably solve this, am I right? |
|
@adrian-carro If you can sort out which parameters you think are the troublesome ones that would be helpful. In general it seems like Dan was doing a lot of "dark magic" by putting all of the code in a single package and then using package private access to implicitly pass around data. This makes it hard to follow the flow of information through the model. Hopefully this new configuration framework will help make this flow more explicit... |
|
So, inside HouseRentalMarket.java, the config file is defined as static so that RENT_GROSS_YIELD can be used inside a static method. The same happens inside HouseSaleMarket with HPI_LOG_MEDIAN and HPI_SHAPE, and inside the Model class itself. By the way, I've seen you've changed the name of some of the parameters: is it to adhere to a certain java naming convention? If that is the case, then I guess I should be aware of that convention and change everything accordingly as we move forward... |
|
@adrian-carro Thanks. Yes, I am slowly changing the naming conventions of variables, fields, methods, etc to match Java conventions. |
|
@adrian-carro I am trying to wire up the new BuyToLetConfig class that I created but can not find where these parameters are actually being used. Has the buy-to-let behavior been commented out? |
|
@davidrpugh are your referring to the two parameters BTL_CHOICE_INTENSITY and BTL_CHOICE_MIN_BANK_BALANCE? I find the first at lines 362 and 443 of the HouseholdBehaviour class; and the second at line 423 of the same class. |
@adrian-carro This PR will significantly improve the configuration of the model. Currently the model configuration file is "secretly" passed around the entire model as a package private field in most objects. I am proposing encapsulating groups of related parameters together in their own config classes and then explicitly passing these config classes as args to the various agent constructors.
The changes that I pushed demonstrate what I have in mind...