Как уменьшить строк кода в этот кусок кода? Количество линий повторяются в updateProfileFlags и fetchProfileObj


Этот метод инициирует новое соединение с бережливость клиента и возвращает объект профиля

 private TResume fetchProfileObj(int userId) {
    try{
        TTransport transport = new TSocket("172.XX.X.XXX”, 9311);  
        transport.open();
        TProtocol protocol = new  TBinaryProtocol(transport);
        Map<String, String> ids = new HashMap<String, String>();
        ids.put("requestId", "settingsService");
        TResumeService.Client resumeServiceClient = new TResumeService.Client(protocol);
        TResume profileObj = resumeServiceClient.getFullActiveProfileFromUserId(ids, 105, userId, "resman5_1");
        transport.close();
        return profileObj;
    } catch(Exception e) {
        e.printStackTrace();
        return null;
    }
}

Этот метод обновляет объект Профиль получил от метода fetchProfile

private void updateProfileFlags(TResume profile,int recruiterJobAlert) {
    try{
        TTransport transport = new TSocket("172.XX.X.XXX", 9321);  
        transport.open();
        TProtocol protocol = new  TBinaryProtocol(transport);
        Map<String, String> ids = new HashMap<String, String>();
        ids.put("requestId", "settingsService");
        TUpdateResume.Client updateResumeClient = new TUpdateResume.Client(protocol);
        profile.getUser().setResdexVisibility(recruiterJobAlert>0?"a":"c");
        updateResumeClient.saveProfile(ids, 105, profile.getProfile(), "now", "resman5_1", null);
        transport.close();
    } catch(Exception e) {
        e.printStackTrace();
    }
}

Сейчас звонит указанных выше способов

public void myTestMethod(){
updateProfileFlags(fetchProfileObj(userId),settings.getRecruiterJobAlert());
}


115
1
задан 29 января 2018 в 09:01 Источник Поделиться
Комментарии
2 ответа

1) выписка дубликата карты в постоянном поле (которое удалить 2 строки из обоих способ) :

private static final Map<String, String> IDS = Collections.singletonMap("requestId", "settingsService");

2) распаковываем создание TResumeService.Client и TUpdateResume.Client в отдельных метода выглядит примерно так :

private TResumeService.Client createService(TTransport transport) {
return new TResumeService.Client(new TBinaryProtocol(transport));
}

3) Не закрывайте TTransport в блоке try, действительно ли исключение, оно никогда не будет закрыто, использовать блок finally для этого.

4) не поймать исключение, чтобы просто использовать printStackTraceэто плохая практика, так как вызов метода не знаю, если (и насколько) метод не удалось. Если методы не могут управлять исключение... они выдали его. Если иметь общие Exception объект, будучи брошенным вокруг вас не устраивает, вы можете поместить его внутрь IOException для примера такой :

try{
TTransport transport = new TSocket("172.XX.X.XXX”, 9311);
transport.open();
// ....
return profileObj;
} catch(Exception e) {
throw new IOException(e);
}

5) использовать пробелы после запятых и добавить немного пространства в следующее выражение (recruiterJobAlert>0?"a":"c" чтобы сделать его более читабельным

6) Поставить строку и 105 и 9311 "магические" значения в правильно именованных констант.

7) бесполезно что-то сохранить в переменной, а затем просто вернуть его, вы должны рассмотреть возможность отмены таких распайка.

В этот момент, Вы будете иметь следующий код :

private static final String RESMAN = "resman5_1";

private static final Map<String, String> IDS = Collections.singletonMap("requestId", "settingsService");

private TResumeService.Client createResumeService(TTransport transport) {
return new TResumeService.Client(new TBinaryProtocol(transport));
}

private TUpdateResume.Client createUpdateService(TTransport transport) {
return new TUpdateResume.Client(new TBinaryProtocol(transport));
}

private TResume fetchProfileObj(int userId) throws IOException {
try{
TTransport transport = new TSocket("172.XX.X.XXX”, PORT);
transport.open();
TResumeService.Client resumeServiceClient = createResumeService(transport);
return resumeServiceClient.getFullActiveProfileFromUserId(IDS, SOME_CONSTANT, userId, RESMAN);
} catch(Exception e) {
throw new IOException(e);
} finally {
transport.close();
}
}

private void updateProfileFlags(TResume profile, int recruiterJobAlert) throws IOException {
try{
TTransport transport = new TSocket("172.XX.X.XXX", PORT);
transport.open();
TUpdateResume.Client updateResumeClient = createUpdateService(transport);
profile.getUser().setResdexVisibility(recruiterJobAlert > 0 ? "a" : "c");
updateResumeClient.saveProfile(IDS, SOME_CONSTANT, profile.getProfile(), "now", RESMAN, null);
} catch(Exception e) {
throw new IOException(e);
} finally {
transport.close();
}
}


Может быть, вы найдете достаточно улучшение, но есть еще 6-7 линии дублируются между 2 методами.
Давайте копать немного больше и пойти работоспособен :

Мы видим, что 2 метода выглядит так :

try{
TTransport transport = new TSocket("172.XX.X.XXX”, PORT);
transport.open();
// something that varies here
} catch(Exception e) {
throw new IOException(e);
} finally {
transport.close();
}

В something that varies может быть абстрагирована с функцией...
Начиная с Java 8 мы можем передавать объекты, которые очень маленькие обертки вокруг функций, они называются lambdaС.
Давайте взглянем на упаковку хранить все эти объекты : https://docs.oracle.com/javase/8/docs/api/java/util/function/package-summary.html
Мы к сожалению не можем использовать Function как он не может бросить исключение... есть Callable интерфейс в Java.утиль.одновременных, но мы не можем использовать, т. к. он не принимает никаких параметров... так что давайте использовать новый функциональный интерфейс, который соответствует нашим потребностям :

public interface ApplicationOverTransport<R> {
R call(TTransport transport) throws Exception;
}

Этот интерфейс будет просто заменить переменную часть, теперь мы можем создать простой метод, который будет использоваться каждый раз при необходимости TTransport, который открывается, сделать некоторые действия и закрытые :

T applyOverTransport(ApplicationOverTransport<T> method) throws IOException {
try{
TTransport transport = new TSocket("172.XX.X.XXX”, PORT);
transport.open();
return method.call(transport);
} catch(Exception e) {
throw new IOException(e);
} finally {
transport.close();
}
}

Используя лямбда-выражения, методы может выглядеть следующим образом :

private TResume fetchProfileObj(int userId) throws IOException {
return applyOverTransport(transport -> {
TResumeService.Client resumeServiceClient = createService(transport);
return resumeServiceClient.getFullActiveProfileFromUserId(IDS, SOME_CONSTANT, userId, RESMAN);
});
}

private void updateProfileFlags(TResume profile, int recruiterJobAlert) throws IOException {
applyOverTransport(transport -> {
TUpdateResume.Client updateResumeClient = createService(transport);
profile.getUser().setResdexVisibility(recruiterJobAlert > 0 ? "a" : "c");
updateResumeClient.saveProfile(IDS, SOME_CONSTANT, profile.getProfile(), "now", RESMAN, null);
return null;
});
}

1
ответ дан 29 января 2018 в 10:01 Источник Поделиться

Я не знаю, сколько строк кода может быть уменьшен, но одно можно сказать точно: всякий раз, когда вы обнаружите ресурсов, которые могут быть закрыты, вы должны использовать Java попытки с ресурсами объекта (начиная с Java 7). Не только эта функция экономит вам close() операцию (одну линию меньше!) но также гарантирует, что ресурс закрыт до конца блока try независимо от того, если она закончилась успешно или нет (компилятор добавляет finally п.). в текущий код, если исключение выдается, Ttransport ресурс не закрыт должным образом.

Я посмотрел на бережливость документации javadoc и действительно с версии 0.10.0, Ttransport автоматически закрывающиеся. Поэтому код на оба метода должны быть

try (TTransport transport = new TSocket("172.XX.X.XXX", 9321)) {
transport.open();
... // do not call close()!!
// do not call close()!! it is added by the compiler
} catch(Exception e) {
e.printStackTrace();
}

Что касается сокращения дублированных строк, я могу предложить, что вы делаете метод создания ids карте. так что если содержимое карте должны быть изменены из-за изменений в требованиях или API, там будет только одно место, чтобы изменить их. создание постоянных переменных вместо встроенных литералы тоже считается лучшим. Это может применяться к другим литералы, используемые в коде

public static final String PROFILE_REQUEST_KEY = "requestId";
public static final String PROFILE_REQUEST_VALUE = "settingsService";
public static final int PROFILE_WHATEVER_105_MEANS = 105;
public static final String PROFILE_WHATEVER_RESMAN5_1_MEANS = "resman5_1";

public Map<String, String> getProfileRequestProperties() {
return Collections.singletonMap(PROFILE_REQUEST_KEY, PROFILE_REQUEST_VALUE);
}

теперь, так как мы ликвидировали close()мы можем вернуть значение прямого вызова к клиенту, исключая создание profileObj ссылка:

return resumeServiceClient.getFullActiveProfileFromUserId(
getProfileRequestProperties(),
PROFILE_WHATEVER_105_MEANS,
userId,
PROFILE_WHATEVER_RESMAN5_1_MEANS);

так, я не уверен, сколько Лок сейчас присутствуют, но им мои глаза, код понятнее и более ремонтопригодна и расширяемым.

1
ответ дан 29 января 2018 в 10:01 Источник Поделиться