Skip to main content
Code Review

Return to Answer

replaced http://programmers.stackexchange.com/ with https://softwareengineering.stackexchange.com/
Source Link

Quoting from here here

and here here

Quoting from here

and here

Quoting from here

and here

Bounty Awarded with 50 reputation awarded by flakes
deleted 4 characters in body
Source Link
Legato
  • 9.9k
  • 4
  • 50
  • 118

IMHO a much better alternative would be SIMON ( Simple Invocation of Methods Over Network), which is activeactively developed and maintained. I use this myself for a multiservermulti-server/multiclientmulti-client system and doesn'tdon't regret theits use of it.

IMHO a much better alternative would be SIMON ( Simple Invocation of Methods Over Network), which is active developed and maintained. I use this myself for a multiserver/multiclient system and doesn't regret the use of it.

IMHO a much better alternative would be SIMON ( Simple Invocation of Methods Over Network), which is actively developed and maintained. I use this myself for a multi-server/multi-client system and don't regret its use.

Source Link
Heslacher
  • 50.9k
  • 5
  • 83
  • 177

In this program there is a single remote server which performs miscellaneous work. Because there is only one server, it seems like a singleton pattern could fit well.

Let us see the definition from Wikipedia:

In software engineering, the singleton pattern is a design pattern that restricts the Instantiation of a class to one object. This is useful when exactly one object is needed to coordinate actions across the system. The concept is sometimes generalized to systems that operate more efficiently when only one object exists, or that restrict the instantiation to a certain number of objects. The term comes from the mathematical concept of a singleton.

So basically you are on the right track but what can't a simple private final object accomplish what the singleton can ?

Furthermore, which disadvantages has using a singleton ?

Quoting from here

Singletons are essentially static classes, relying on one or more static methods and properties. All things static present real, tangible problems when you try to do Unit Testing because they represent dead ends in your code that cannot be mocked or stubbed. As a result, when you test a class that relies on a Singleton (or any other static method or class) you are not only testing that class but also the static method or class.

and here

It is because of this specific design choice that the pattern introduces several potential long-term problems:

  • Inability to use abstract or interface classes;
  • Inability to subclass;
  • High coupling across the application (difficult to modify);
  • Difficult to test (can't fake/mock in unit tests);
  • Difficult to parallelize in the case of mutable state (requires extensive locking);
  • and so on.

None of these symptoms are actually endemic to single instances, just the Singleton pattern.

What can you do instead? Simply don't use the Singleton pattern.

So basically this boils down, if you really need to use it then use it, but make sure you understand the possible pitfalls and possible upcoming problems.

That being said, let me review some of your code.


public class Configuration {
 public static final int COMPUTE_REMOTE_PORT = 8900;
 public static final String COMPUTE_REMOTE_ID = "COMPUTE_REMOTE_ID";
 public static final String COMPUTE_REMOTE_HOST = "localhost";
} 

What will happen if the port or the remote host needs to be changed ? You will need to recompile this class because all members are static and can't be changed. How about testability of the server /client code ?

Using this setup also restricts the server to be on the same computer than the client(s).


Based on how you have implemented the ComputeService and the ComputeInterface I would like to suggest that you add another method to the ComputeTask interface returning if the task really has been processed. Right now if the ComputeService can't connect to the server the returned ComputeTask object doesn't tell the client that it hadn't been processed.

The private static boolean connected() method doesn't throw a RemoteException and therefor either the isConnected() method of the ComputeInterface shouldn't have a throws RemoteException or better, let the connected() really throw that exception.

Right now if this method returns false the client doesn't know what the problem is.

In addition, I would name the connected() method just isConnectedToServer() or something along this line. But nevertheless this method is doing to much because it is doing a registry lookup so to say it is connecting to the server, but is read like a simple "property getter" .

I would change isConnected() method like so

@Override
public synchronized boolean isConnected() throws RemoteException{
 if (computeServer == null)
 {
 return false;
 }
 return computeServer.isConnected();
}

Then I would replace the connected() method by a, to the ComputeInterface added, boolean connect() method which does exactly what the name implies like so

@Override
public synchronized boolean connect() throws RemoteException{
 if (computeServer == null) {
 try{
 Registry reg = LocateRegistry.getRegistry(Configuration.COMPUTE_REMOTE_HOST, Configuration.COMPUTE_REMOTE_PORT);
 computeServer = (ComputeInterface) reg.lookup(Configuration.COMPUTE_REMOTE_ID);
 System.out.println("New Connection to Compute Server");
 }
 catch (RemoteException | NotBoundException e){
 System.err.print(e);
 computeServer = null;
 throw e;
 }
 }
 return true;
}

and if you want to add some kind of authentication, I would add a boolean connect(someparameters) method to the ComputeInterface which then is called instead of just returning true.


The most recent question for me is if you really should use RMI for this application. RMI has some disadvantages like

  • it is using blocking IO
  • if you want to use callbacks, you will likely come into trouble if a router or firewall is set up
  • and some more

IMHO a much better alternative would be SIMON ( Simple Invocation of Methods Over Network), which is active developed and maintained. I use this myself for a multiserver/multiclient system and doesn't regret the use of it.

lang-java

AltStyle によって変換されたページ (->オリジナル) /