Совершенный код << 1 ... 18 19 20 21 22 ... 24 >> 3.07.2017 / 09:36 | | Ginosaji Пользователь Сейчас: Offline
Имя: Игорь Откуда: Воронеж Регистрация: 30.01.2010
| Портянки: Member myMember = EventUtils.getMember(model.getMembers(), myUserID);
MemberStatus myStatus;
if (myMember != null) {
myStatus = MemberStatus.from(myMember.getStatus());
} else {
myStatus = MemberStatus.UNDEFINED;
}
if (useMyStatusInTask && myStatus == MemberStatus.TASK_DONE) {
date = myMember.getStatusChangeDate();
} else {
date = model.getStatusChangeDate();
}
Заменить на тернарный оператор: Member myMember = EventUtils.getMember(model.getMembers(), myUserID);
MemberStatus myStatus = myMember == null ? MemberStatus.UNDEFINED : MemberStatus.from(myMember.getStatus());
date = (useMyStatusInTask && myStatus == MemberStatus.TASK_DONE) ? myMember.getStatusChangeDate() : model.getStatusChangeDate();
И s/events.size() == 0/events.isEmpty()/
|
3.07.2017 / 10:28 | | Naik Пользователь Сейчас: Offline
Имя: %name% Регистрация: 14.03.2010
| Ginosaji, Тернарники в общем случае только ухудшают читабельность и не расширяемы емли нужно дописать парочку else if
|
3.07.2017 / 10:42 | | aNNiMON Супервизор Сейчас: Offline
Имя: Витёк Регистрация: 11.01.2010
| Naik, кстати, согласен насчёт тернарников. С ними код растёт в ширину, а это куда хуже для чтения. Для коротких выражений ещё можно, но последнее составное условие для тернарника явно перебор. Можно, конечно, форматировать. Member myMember = EventUtils.getMember(model.getMembers(), myUserID);
MemberStatus myStatus = myMember == null
? MemberStatus.UNDEFINED
: MemberStatus.from(myMember.getStatus());
date = (useMyStatusInTask && myStatus == MemberStatus.TASK_DONE)
? myMember.getStatusChangeDate()
: model.getStatusChangeDate();
__________________
let live Изменено aNNiMON (3.07 / 10:44) (всего 1 раз) |
3.07.2017 / 10:51 | | Naik Пользователь Сейчас: Offline
Имя: %name% Регистрация: 14.03.2010
| aNNiMON, строчки с 22 по 57 превратятся в date = getDate(context, i, model);А как же то, что там используются переменные labelFormatter, todayLocalDate, oldLocalDate, oldDateString?
Сейчас даты создаются один раз, а будут каждую итерацию, а обьект oldDateString и подобные обьекты которые зависят от других элементов списка и динамически меняются придется передавать тоже в getDate(...)
Изменено Naik (3.07 / 10:51) (всего 1 раз) |
3.07.2017 / 11:18 | | Naik Пользователь Сейчас: Offline
Имя: %name% Регистрация: 14.03.2010
| Без InsertSectionType все получается намного проще, вот вставка дат в обьекты где просто нужно ориентироваться на какую-то дату Открыть спойлер Закрыть спойлер
public static List<BaseModel> insertSectors(Context context, List<? extends ISectioned> src,
boolean insertMonthSectors,
LabelFormatter labelFormatter) {
if (src == null || src.size() == 0) return Collections.emptyList();
[code] List<BaseModel> models = new ArrayList<>((int) (src.size()));
String oldDateString = null;
LocalDate oldLocalDate = null;
ISectioned model;
int oldMonthOfYear = -1;
int oldYear = -1;
final int currentYear = new LocalDate().getYear();
for (int i = 0; i < src.size(); i++) {
model = src.get(i);
Date date = model.getSectionDate();
if (date != null) {
date = DateTimeUtils.fromUTC(date);
LocalDate localDate = new LocalDate(date);
String dateString = labelFormatter.format(context, model, date);
if (insertMonthSectors) {
final int monthOfYear = localDate.getMonthOfYear();
final int year = localDate.getYear();
if (i != 0 && (monthOfYear != oldMonthOfYear || year != oldYear)) {
models.add(new EventListMonthSectorHeader(-(year + 13 + monthOfYear), localDate, year != currentYear));
}
oldMonthOfYear = monthOfYear;
oldYear = year;
}
if (i == 0 || (!localDate.equals(oldLocalDate) && !dateString.equals(oldDateString))) {
models.add(new SimpleItem<>(dateString, -date.getTime()));
}
oldLocalDate = localDate;
oldDateString = dateString;
}
models.add(model);
}
return models;
}
[/code]
Изменено Naik (3.07 / 11:22) (всего 1 раз) |
3.07.2017 / 11:41 | | Ginosaji Пользователь Сейчас: Offline
Имя: Игорь Откуда: Воронеж Регистрация: 30.01.2010
| Naik, дело вкуса. Я пишу if else цепочки, только при сравнении разнородных вещей, а если можно, то либо тернарник форматированный, либо switch для однородных. Ну и ещё на мой вкус, я не люблю слишком длинные if блоки, да ещё и с большой вложенностью, потому что прокрутив вниз, можно и забыть, что там вверху условия какие-то. for (int i = 0; i < src.size(); i++) {
model = src.get(i);
Date date = model.getSectionDate();
if (date != null) {
date = DateTimeUtils.fromUTC(date);
LocalDate localDate = new LocalDate(date);
String dateString = labelFormatter.format(context, model, date);
if (insertMonthSectors) {
final int monthOfYear = localDate.getMonthOfYear();
final int year = localDate.getYear();
if (i != 0 && (monthOfYear != oldMonthOfYear || year != oldYear)) {
models.add(new EventListMonthSectorHeader(-(year + 13 + monthOfYear), localDate, year != currentYear));
}
oldMonthOfYear = monthOfYear;
oldYear = year;
}
if (i == 0 || (!localDate.equals(oldLocalDate) && !dateString.equals(oldDateString))) {
models.add(new SimpleItem<>(dateString, -date.getTime()));
}
oldLocalDate = localDate;
oldDateString = dateString;
}
models.add(model);
}
Я бы сделал: for (int i = 0; i < src.size(); i++) {
model = src.get(i);
Date date = model.getSectionDate();
if (date == null) {
models.add(model);
continue;
}
date = DateTimeUtils.fromUTC(date);
LocalDate localDate = new LocalDate(date);
String dateString = labelFormatter.format(context, model, date);
if (insertMonthSectors) {
final int monthOfYear = localDate.getMonthOfYear();
final int year = localDate.getYear();
if (i != 0 && (monthOfYear != oldMonthOfYear || year != oldYear)) {
models.add(new EventListMonthSectorHeader(-(year + 13 + monthOfYear), localDate, year != currentYear));
}
oldMonthOfYear = monthOfYear;
oldYear = year;
}
if (i == 0 || (!localDate.equals(oldLocalDate) && !dateString.equals(oldDateString))) {
models.add(new SimpleItem<>(dateString, -date.getTime()));
}
oldLocalDate = localDate;
oldDateString = dateString;
models.add(model);
}
|
24.07.2017 / 16:24 | | Death Пользователь Сейчас: Offline
Имя: Смерть Регистрация: 31.07.2015
| Скажите что не так в коде написано. ООП, или вообще логика. Что лишние, а что стоило бы добавить? Пост #490975 __________________
Смерть правит миром |
24.07.2017 / 16:40 | | aNNiMON Супервизор Сейчас: Offline
Имя: Витёк Регистрация: 11.01.2010
| 2. Лихой синглтон Лихой синглтон public class MyGame extends Game {
public static MyGame game;
...
class ApplicationLauncher {
...
MyGame.game = new MyGame();
Лихо ты сделал синглтон. Коль так, мог бы хоть не разбрасывать его по куче классов, а инициализировать в одном месте, как в нормальном синглтоне:
public class MyGame extends Game{
public static MyGame game;
public static MyGame getInstance() {
if (game == null) {
game = new MyGame();
}
return game;
}
...
new LwjglApplication(MyGame.getInstance(), cnf);
Ну, ООП ж знаешь, должен же понимать, как синглтон делать и почему он -- антипаттерн.
И, я гляжу, ты любишь такое использовать. В ApplicationLauncher, Asset тоже статическая ссылка на объект.load() load() Во многих классах есть метод load(), можно было это в интерфейс какой-нибудь закинуть. Как минимум для улучшения читабельности, чтоб сразу понять, что этот класс представляет объект, который может загружаться, то есть ресурс какой-то, текстура, карта и т.д.Инициализаторы вне конструкторов Инициализаторы вне конструкторов class World {
public ArrayList<WorldObject> objectes = new ArrayList<>();
Всегда против такого. Заполнение полей должно происходить в конструкторе, а не при объявлении. Вот этот код автоматически влепит objectes = new ArrayList<>(); в каждый конструктор.addStatic/addDynamic addStatic/addDynamic public void add(WorldObject ob, boolean stat){
this.objectes.add(ob);
if(stat){
gridObject.addStatic(ob);
}else{
gridObject.addDynamic(ob);
}
}
А не лень каждый раз вспоминать, какой объект статичный, а какой динамичный? Пометил бы классы объектов соответствующими интерфейсами или отнаследовал от классов (например, StaticGridObject/DynamicGridObject), а в WorldGridObject сделал бы два перегруженных метода:
public void add(StaticGridObject ob) { .. }
public void add(DynamicGridObject ob) { .. }
и тогда не нужно было делать такие проверки и передавать флажки.Повторяющийся код - зло. Повторяющийся код - зло. Повторяющийся код - зло. Повторяющийся код - зло. Повторяющийся код - зло. Повторяющийся код - зло. Повторяющийся код - зло. Взгляни на методы addStatic и addDynamic класса WorldGridObject и найди десять отличий. Отличие лишь в результирующем массиве, куда будут положены объекты. Взгляни на остальные методы этого класса и ты увидишь 8 одинаковых строк в каждом из них? Тебе не лень было их писать? Уже на третьем методе можно было бы задуматься об этом и вынести, наконец, в новый метод.fin fin В остальном вроде всё нормально. Одиночные классы из пакетов можно было бы выделить в какой-нибудь пакет screens. __________________
let live Изменено aNNiMON (24.07 / 16:46) (всего 7 раз) |
24.07.2017 / 16:59 | | Death Пользователь Сейчас: Offline
Имя: Смерть Регистрация: 31.07.2015
| aNNiMON, Спасибо. Я вот эту сетку объектов из книги взял. Книга по gamedev для андроид. Прям с самого начала хотел сделать, что бы просто объект добовлять, а там внутри уже будет typeObject unum-перечесление, ну или хотя бы inte typeObject и статичные финальные DynamicObject и StaticObject. Сама сетка - вот проблема. А так из моего это только статичные переменные прямым доступом, как я понял.
__________________
Смерть правит миром |
<< 1 ... 18 19 20 21 22 ... 24 >> Всего сообщений: 233 Фильтровать сообщения Поиск по теме Файлы топика (7)
|