use TransportPaths class instead of static path variables #354
Conversation
| import java.nio.file.Path; | ||
|
|
||
| @Getter | ||
| @Setter |
There was a problem hiding this comment.
we don't want setters because once the paths are initialized, we don't want them to change, and we probably don't want getters either since the fields are public.
| @Setter | ||
| public class TransportPaths{ | ||
| @Getter | ||
| public Path fromClient; |
| @@ -3,6 +3,11 @@ | |||
| import static java.util.logging.Level.INFO; | |||
| import static java.util.logging.Level.SEVERE; | |||
|
|
|||
| import android.content.Context; | |||
There was a problem hiding this comment.
why were these imports added? you probably want to run the optimized imports helper.
| @@ -181,4 +183,8 @@ TransportWifiDirectService getService() { | |||
| return TransportWifiDirectService.this; | |||
| } | |||
| } | |||
|
|
|||
| public void setTransportPaths(TransportPaths transportPaths) { | |||
There was a problem hiding this comment.
i don't think we want this. we might end up with a race condition. TransportWifiDirectService should probably make it's own TransportPaths in onStartCommand()
| @@ -42,12 +43,12 @@ public RpcServer(BundleExchangeServiceImpl.BundleExchangeEventListener listener) | |||
| this.listener = listener; | |||
| } | |||
|
|
|||
| public void startServer(Context context) { | |||
| public void startServer(Context context, TransportPaths transportPaths) { | |||
There was a problem hiding this comment.
do you still need to pass the context?
| import java.nio.file.Path; | ||
| import java.util.logging.Logger; | ||
|
|
||
| import static java.util.logging.Level.SEVERE; | ||
|
|
||
| @Getter |
There was a problem hiding this comment.
you can get rid of the getters too.
| @Getter | ||
| public Path toServer; | ||
| public final Path toServer; | ||
|
|
||
| public TransportPaths(Path rootDir){ | ||
| this.toServer = rootDir.resolve("BundleTransmission/server"); |
There was a problem hiding this comment.
since toServer is the same as fromClient, you should pick only one and delete the other, otherwise bug fixes to one might not propagate to the other.
BundleTransport/app/src/main/java/net/discdd/bundletransport/TransportWifiDirectService.java
Show resolved
Hide resolved
| public Path getToClient(){ return clientPath; } | ||
|
|
||
| public Path getToServer(){ return serverPath; } | ||
|
|
||
| public Path getFromClient(){ return serverPath; } | ||
|
|
||
| public Path getFromServer(){ return clientPath; } |
There was a problem hiding this comment.
i don't think you should have these getters. the code will be confusing with an inconsistent use of getFromServer and getToClient for the same path. just make the member variables public.
| Files.createDirectories(serverPath); | ||
| } | ||
| } catch (Exception e) { | ||
| logger.log(SEVERE, "Failed to get inventory", e); |
There was a problem hiding this comment.
i'm not sure what this message means. perhaps "Failed to create transport storage directories" is better?
| private final Path clientPath; | ||
| private final Path serverPath; |
There was a problem hiding this comment.
make public. change to toClientPath and toServerPath to get the directions clear.
| import android.os.Handler; | ||
| import android.os.Looper; | ||
| import android.widget.Toast; |
There was a problem hiding this comment.
why did these get added? did you use the optimize imports tool?
No description provided.