search

lithin9 / Unturned-Custom-Plugin

Other
0 stars 0 forks source link

PlayerState.cs needs to be refactored #174

Open prestonvanloon opened 8 years ago

prestonvanloon commented 8 years ago

Why so many inner classes? This is an antipattern and you're going to end up with a huge class that's hard to maintain.

jcvl92 commented 8 years ago

Because the classes are private to PlayerState. They will never be used or referenced outside of PlayerState, they are just there for better organization. The classes themselves are components of PlayerState and use private variables of PlayerState. It would not be possible to remove them without working around their dependence on these private variables.

prestonvanloon commented 8 years ago

Can't you make it package only? I still don't see a valid reason to have inner classes here.

This is not better organization if you have to search through a mammoth file to add functionality to a small piece.

jcvl92 commented 8 years ago

On re-reading the code, I think it can be segmented into different classes. I think I got rid of intra-class dependencies. Moving this to phase 1.

prestonvanloon commented 8 years ago

You can declare these classes internal so they can't be accessed outside of the assembly

jcvl92 commented 8 years ago

Yeah, access control wasn't the issue. It was parent/inner class member sharing that was the issue. I am pretty sure I am not doing that anymore, though. I will change it with the next update.