CORPORATION: add issueNewSharesCooldown and makesMaterial #774
Conversation
|
What about adding both makesMaterials and makes Products to getIndustryData now? Then they're both in the right place, and together, and the only downside is a little duplication. |
|
“Duplication may be the root of all evil in software.” - Robert C. Martin , Clean Code |
|
Yeah I agree with @d0sboots here, they make more sense on getIndustryData since they are applicable to all divisions within an industry. getDivision should just have the info that is specific to that one division within the industry. I think for now duplicating it is fine, but we should remove the old out of place entry on getDivision in the future, I guess at the next API break. |
|
with them on getIndustryData i dont see a reason to add makesMaterials to getDivision at all |
Yeah, that was the idea. |
It's much better in this case than making a permanently suboptimal API choice because of a prior bad API choice, or forcing a breaking API change during a minor version bump. Evolving the BB API has different challenges than other things because it's a push model, not a pull model. (Technically, I suppose we could keep every version available in Steam, but I'm not sure how many it allows, and it's really not designed for that.) |
moved makes Product/Materials to getIndustryData removed makesMaterials from getDivision to reduce code dupe run doc
It was unused, and was also initialized incorrectly, referencing `this.producedMaterials` prior to object initialization when it's an empty array
|
Just pushed a quick commit to remove the duplicate makesMaterials from Division, which was not used anywhere. |
Caldwell-74
deleted the
CORPORATION-add-issueNewSharesCooldown-to-getCorporation
branch
adds issueNewSharesCooldown to getCorporation and makesMaterials to getDivision
makesMaterials should be added to getIndustryData but makesProducts is on getDivision already so it wouldnt makes sense to split it
moving both would be an API break i didnt want to make for such a detail but i would put it on the list for the next bigger Update
closes #506