2 Add code review as factor
source | link

I would just remove these variables and private methods altogether. Here's my refactor:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    return cryptoService.encryptResponse(
        serviceClient.handle(router.getDestination().getUri(),
        cryptoService.decryptRequest(encryptedRequest, byte[].class),
        cryptoService.getEncryptionInfoForDefaultClient()));
  }
}

For private method, e.g. router.getDestination().getUri() is clearer and more readable than getDestinationURI(). I would even just repeat that if I use the same line twice in the same class. To look at it another way, if there's a need for a getDestinationURI(), then it probably belongs in some other class, not in SomeBusinessProcess class.

For variables and properties, the common need for them is to hold values to be used later in time. If the class has no public interface for the properties, they probably shouldn't be properties. The worst kind of class properties use is probably for passing values between private methods by way of side effects.

Anyway, the class only needs to do process() and then the object will be thrown away, there's no need to keep any state in memory. Further refactor potential would be to take out the CryptoService out of that class.

Based on comments, I want to add this answer is based on real world practice. Indeed, in code review, the first thing that I'd pick out is to refactor the class and move out the encrypt/decrypt work. Once that's done, then I'd ask if the methods and variables are needed, are they named correctly and so on. The final code will probably closer to this:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;

  public Response process(Request request) {
    return serviceClient.handle(router.getDestination().getUri());
  }
}

With above code, I don't think it needs further refactor. As with the rules, I think it takes experience to know when and when not to apply them. Rules are not theories that are proven to work in all situations.

Code review on the other hand has real impact on how long before a piece of code can pass. My trick is to have less code and make it easy to understand. A variable name can be a point of discussion, if I can remove it reviewers wouldn't even need to think about it.

I would just remove these variables and private methods altogether. Here's my refactor:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    return cryptoService.encryptResponse(
        serviceClient.handle(router.getDestination().getUri(),
        cryptoService.decryptRequest(encryptedRequest, byte[].class),
        cryptoService.getEncryptionInfoForDefaultClient()));
  }
}

For private method, e.g. router.getDestination().getUri() is clearer and more readable than getDestinationURI(). I would even just repeat that if I use the same line twice in the same class. To look at it another way, if there's a need for a getDestinationURI(), then it probably belongs in some other class, not in SomeBusinessProcess class.

For variables and properties, the common need for them is to hold values to be used later in time. If the class has no public interface for the properties, they probably shouldn't be properties. The worst kind of class properties use is probably for passing values between private methods by way of side effects.

Anyway, the class only needs to do process() and then the object will be thrown away, there's no need to keep any state in memory. Further refactor potential would be to take out the CryptoService out of that class.

I would just remove these variables and private methods altogether. Here's my refactor:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    return cryptoService.encryptResponse(
        serviceClient.handle(router.getDestination().getUri(),
        cryptoService.decryptRequest(encryptedRequest, byte[].class),
        cryptoService.getEncryptionInfoForDefaultClient()));
  }
}

For private method, e.g. router.getDestination().getUri() is clearer and more readable than getDestinationURI(). I would even just repeat that if I use the same line twice in the same class. To look at it another way, if there's a need for a getDestinationURI(), then it probably belongs in some other class, not in SomeBusinessProcess class.

For variables and properties, the common need for them is to hold values to be used later in time. If the class has no public interface for the properties, they probably shouldn't be properties. The worst kind of class properties use is probably for passing values between private methods by way of side effects.

Anyway, the class only needs to do process() and then the object will be thrown away, there's no need to keep any state in memory. Further refactor potential would be to take out the CryptoService out of that class.

Based on comments, I want to add this answer is based on real world practice. Indeed, in code review, the first thing that I'd pick out is to refactor the class and move out the encrypt/decrypt work. Once that's done, then I'd ask if the methods and variables are needed, are they named correctly and so on. The final code will probably closer to this:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;

  public Response process(Request request) {
    return serviceClient.handle(router.getDestination().getUri());
  }
}

With above code, I don't think it needs further refactor. As with the rules, I think it takes experience to know when and when not to apply them. Rules are not theories that are proven to work in all situations.

Code review on the other hand has real impact on how long before a piece of code can pass. My trick is to have less code and make it easy to understand. A variable name can be a point of discussion, if I can remove it reviewers wouldn't even need to think about it.

1
source | link

I would just remove these variables and private methods altogether. Here's my refactor:

public class SomeBusinessProcess {
  @Inject private Router router;
  @Inject private ServiceClient serviceClient;
  @Inject private CryptoService cryptoService;

  public EncryptedResponse process(EncryptedRequest encryptedRequest) {
    return cryptoService.encryptResponse(
        serviceClient.handle(router.getDestination().getUri(),
        cryptoService.decryptRequest(encryptedRequest, byte[].class),
        cryptoService.getEncryptionInfoForDefaultClient()));
  }
}

For private method, e.g. router.getDestination().getUri() is clearer and more readable than getDestinationURI(). I would even just repeat that if I use the same line twice in the same class. To look at it another way, if there's a need for a getDestinationURI(), then it probably belongs in some other class, not in SomeBusinessProcess class.

For variables and properties, the common need for them is to hold values to be used later in time. If the class has no public interface for the properties, they probably shouldn't be properties. The worst kind of class properties use is probably for passing values between private methods by way of side effects.

Anyway, the class only needs to do process() and then the object will be thrown away, there's no need to keep any state in memory. Further refactor potential would be to take out the CryptoService out of that class.