jeudi 22 décembre 2016

How to write a method using single responsibility princpile?

I have a below class in which isValid method is being called.

  • I am trying to extract few things from Record object in the isValid method. And then I am validating few of those fields. If they are valid, then I am populating the holder map with some additional fields and then I am populating my DataHolder builder class and finally return the DataHolder class back.
  • If they are not valid, I am returning null.

Below is my class:

public class ProcessValidate extends Validate {
  private static final Logger logger = Logger.getInstance(ProcessValidate.class);

  @Override
  public DataHolder isValid(String processName, Record record) {
    Map<String, String> holder = (Map<String, String>) DataUtils.extract(record, "holder");
    String deviceId = (String) DataUtils.extract(record, "deviceId");
    Integer payId = (Integer) DataUtils.extract(record, "payId");
    Long oldTimestamp = (Long) DataUtils.extract(record, "oldTimestamp");
    Long newTimestamp = (Long) DataUtils.extract(record, "newTimestamp");
    String clientId = (String) DataUtils.extract(record, "clientId");

    if (isValidClientIdDeviceId(processName, deviceId, clientId) && isValidPayId(processName, payId)
        && isValidHolder(processName, holder)) {
      holder.put("isClientId", (clientId == null) ? "false" : "true");
      holder.put("isDeviceId", (clientId == null) ? "true" : "false");
      holder.put("abc", (clientId == null) ? deviceId : clientId);
      holder.put("timestamp", String.valueOf(oldTimestamp));

      DataHolder dataHolder =
          new DataHolder.Builder(record).setClientId(clientId).setDeviceId(deviceId)
              .setPayId(String.valueOf(payId)).setHolder(holder).setOldTimestamp(oldTimestamp)
              .setNewTimestamp(newTimestamp).build();
      return dataHolder;
    } else {
      return null;
    }
  }

  private boolean isValidHolder(String processName, Map<String, String> holder) {
    if (MapUtils.isEmpty(holder)) {
      // send metrics using processName
      logger.logError("invalid holder is coming.");
      return false;
    }
    return true;
  }

  private boolean isValidpayId(String processName, Integer payId) {
    if (payId == null) {
      // send metrics using processName     
      logger.logError("invalid payId is coming.");
      return false;
    }
    return true;
  }

  private boolean isValidClientIdDeviceId(String processName, String deviceId, String clientId) {
    if (Strings.isNullOrEmpty(clientId) && Strings.isNullOrEmpty(deviceId)) {
      // send metrics using processName     
      logger.logError("invalid clientId and deviceId is coming.");
      return false;
    }
    return true;
  }
}

Is my isValid method doing lot of things? Can it be broken down in multiple parts? Or is there any better way to write that code?

Also I don't feel great with the code I have in my else block where I return null if record is not valid. I am pretty sure it can written in much better way.

Aucun commentaire:

Enregistrer un commentaire