Смысл и разделение в рефакторинге

Смысл и разделение в рефакторинге

У меня был этот код в моем классе

IAdInfo adInfo;
......
// ad unit ID
String adUnitId = null;
if (!StringUtils.isEmpty(adInfo.getHost()) && !StringUtils.isEmpty(adInfo.getZoneId())) {
    adUnitId = adInfo.getHost() + "," + adInfo.getZoneId();
} else if (!StringUtils.isEmpty(adInfo.getAdUnitId())) {
    adUnitId = adInfo.getAdUnitId();
}

проблема в том, что IAdInfo больше не содержит getHost() и getZoneId(), поскольку они были перемещены в соответствующий интерфейс IAudioAdInfo.

Поэтому я изменил его на это:

// ad unit ID
String adUnitId = null;
if (adInfo instanceof IAudioAdInfo) {
    IAudioAdInfo audioAdInfo = (IAudioAdInfo) adInfo;
    if (!StringUtils.isEmpty(audioAdInfo.getHost()) && !StringUtils.isEmpty(audioAdInfo.getZoneId())) {
        adUnitId = audioAdInfo.getHost() + "," + audioAdInfo.getZoneId();
    } else if (!StringUtils.isEmpty(adInfo.getAdUnitId())) {
        adUnitId = adInfo.getAdUnitId();
    }
} else {
    if (!StringUtils.isEmpty(adInfo.getAdUnitId())) {
        adUnitId = adInfo.getAdUnitId();
    }
}

Но это было слишком коряво, поэтому я создал локальные переменные, чтобы сделать его более читабельным:

// ad unit ID
String adUnitId = null;
if (adInfo instanceof IAudioAdInfo) {
    IAudioAdInfo audioAdInfo = (IAudioAdInfo) adInfo;
    String host = audioAdInfo.getHost();
    String zoneId = audioAdInfo.getZoneId();

    if (!StringUtils.isEmpty(host) && !StringUtils.isEmpty(zoneId)) {
        adUnitId = host + "," + zoneId;
    } else if (!StringUtils.isEmpty(adInfo.getAdUnitId())) {
        adUnitId = adInfo.getAdUnitId();
    }
} else {
    if (!StringUtils.isEmpty(adInfo.getAdUnitId())) {
        adUnitId = adInfo.getAdUnitId();
    }
}

И тут я понял, что на самом деле нужно предоставить только adUnitId и я могу перенести его в реализацию интерфейса.

Вот что я получил в своем первоначальном классе:

String adUnitId = null;
if (!StringUtils.isEmpty(adInfo.getAdUnitId())) {
    adUnitId = adInfo.getAdUnitId();
}

И в моей реализации интерфейса IAudioAdInfo:

@Override
public String getAdUnitId() {
    if (!StringUtils.isEmpty(mHost) && StringUtils.isEmpty(mZoneId)) {
        return mHost + "," + mZoneId;
    } else {
        return super.getAdUnitId();
    }
}

Несколькими строками ниже я увидел, что уже есть проверка на пустое значение adUnitId, поэтому я упростил ее:

String adUnitId = adInfo.getAdUnitId();

if (StringUtils.isEmpty(adUnitId)) {
    LogHelper.e(LogHelper.TAG, "AdReporter: no host/zoneId or adUnitId, can't report");
    return;
}

Надеюсь, это поможет вам в рефакторинге и написании лучшего кода.