EsotericSoftware / kryonet

TCP/UDP client/server library for Java, based on Kryo
BSD 3-Clause "New" or "Revised" License
1.82k stars 419 forks source link

Make the fields in Client and Server protected instead of private #16

Closed ghost closed 10 years ago

ghost commented 10 years ago

From nosacha...@gmail.com on December 28, 2011 19:21:12

All class fields in both Client and Server classes are private, which prevents the classes to be extended in many ways. For example I need to determine whether a given server is started or not. I can't, there is no isConnected() method in the Server.

So I thought, no problem, I'll just extend it. Can't either. All fields are private, so I can't access the shutdown field which tracks exactly that in the Server class.

Instead of private, all fields should be protected so developers can tweak things without messing around with the original source code (which gets replaced when a new version comes out). :)

Thanks! Eduardo

Original issue: http://code.google.com/p/kryonet/issues/detail?id=15

ghost commented 10 years ago

From yury.suk...@gmail.com on February 20, 2012 23:37:58

Hello dear Eduardo. Making all private fields as protected in general is a bad idea, because this makes developers to maintain all these fields in all versions. Let us care about them and do not request doing this :). If they will not maintain them you still can't use them, as far as they may change from version to version.

Such types of problems usually resolved in two ways:

  1. if you know there is an information about a state of object what will not change from version to version by meaning (your example with state of thread) and it's ok to show this information to public - this is a good candidate to feature request and extending the class API.
  2. if you need to extend functionality without accessing hidden state - decorate the class without subclassing. Your sample problem can be resulved in this way as well.

Thanks :).

ghost commented 10 years ago

From nosacha...@gmail.com on February 23, 2012 10:29:29

Oh yeah absolutely. I know all your points are valid, my only request to actually making them protected would be for me to extend the class' API myself, but if you would be willing to extend it permanently that would be ideal. All I needed really is a isConnected method. Right now in order to determine this without reflection (which is bad since it my break from version to version) I have to check whether the ports I have configured are currently in use or not, which is really a hack.

I'll enter a request for the API extensions I have in mind (not many really) and we can go from there, what do you think?

Thanks! Eduardo

ghost commented 10 years ago

From nathan.s...@gmail.com on April 25, 2012 20:07:37

What is the exact need in the API? Server#isConnected wouldn't be a good method name, as it would be confusing with Connection#isConnected and wouldn't apply to Client. Do you want an isRunning() method that would return true if the update thread is running? Typical KryoNet usage starts the server or client and doesn't stop it again. It is fine to leave it running even if the server or client are closed and bound again (Server#bind) or reconnected (Client#connect). If you do need to track the state of the update thread, you could set a boolean when you start/stop it.

Status: WontFix