hubert
41 уровень

Говнокод #1

Пост из группы Архив info.javarush.ru
3681 участников
boolean is_admin; // something Boolean b = new Boolean( is_admin ); if( b.toString().length() == 4 ) { // something... } // something
Комментарии (29)
  • популярные
  • новые
  • старые
Для того, что бы оставить комментарий вы должны авторизироваться
IvanDurov 25 уровень
20 апреля 2014, 19:55
return 5 < age ? false : true

Правильно:
return age <= 5
alexnjc 31 уровень
20 апреля 2014, 03:49
boolean is_admin;
// something
Boolean b = new Boolean( is_admin );
if( b.toString().length() == 4 ) {
   // something...
}
// something

Поскольку b.toString().length() == 4
соответствует только true и null, но из примитивного boolean нельзя получить null.
Все упрощается так:
boolean is_admin;
// something
if(is_admin) {
   // something...
}
// something
Predict_it 21 уровень
20 апреля 2014, 02:29
int var;
int var2;
int var3;
int var4;

int var5 = var + var2 + var3 + var4;
L2CCCP 9 уровень, Калининград
19 апреля 2014, 02:21
public class Access
{
	private boolean isAdmin = false; //Default is false

	public void setAdmin(boolean isAdmin)
	{
		this.isAdmin = isAdmin;
	}

	public boolean getAdmin()
	{
		return isAdmin;
	}

	public void doSomething()
	{
		if(getAdmin())
		{
			//do something...
		}
	}
}
Timur 20 уровень
19 апреля 2014, 16:20
Я поставил минус так как если бы я использовал твой класс то в итоге было бы что-то типо этого:
...
Access access = new Access(); // создаем доступ
access.setAdmin(true); // устанавливаем доступ админом
boolean admin = access.getAdmin(); // получаем админа

...

// если получаем админа
if (access.getAdmin()) {
    // что-то делаем
}
...

access.setAdmin(false); // устанавливаем доступ НЕ админом
boolean admin2 = access.getAdmin(); // получаем НЕ админа
...


Если я вызываю например у объекта Button методы:
button.getText(); // я как правило ожидаю в ответ какой-либо String.
button.setText("blabla"); // я ожидаю что у объекта установиться мой String.
button.isEnabled(); // я ожидаю что объект вернет мне boolean.


Если я не правильно юзаю твой код, то интересно было бы узнать как нужно правильно.
L2CCCP 9 уровень, Калининград
19 апреля 2014, 18:38
Если Вы слышали о таком принципе ООП как инкапсуляция Вы поймете мой пример :)
Groomsh 33 уровень, Москва
19 апреля 2014, 18:57
А зачем эта строка, если не секрет?)
В смысле — зачем явное указание, учитывая, что есть дефолтная инициализация переменных?)
private boolean isAdmin = false; //Default is false
Timur 20 уровень
19 апреля 2014, 19:18
Наверно из-за того что во многих книгах советуют явно в некоторых случаях инициализировать переменные для лучшей читабельности например
счетчик советуют всегда инициализировать явно.
Timur 20 уровень
19 апреля 2014, 19:21
Я слышал и прочитал не одну книгу и смотрел много всяких исходников поэтому я и спросил) Тут вопрос насчет читабельности а не инкапсуляции мне кажется если сделать хотя бы из getAdmin() -> isAdmin() и подумать еще над именем класса читаймость бы увеличилась.

Вы поймете мой пример :)
после чтение кода да а если я не хочу лезть в ваш класс а хочу просто его использовать а не тратить свое время на его изучение?
L2CCCP 9 уровень, Калининград
20 апреля 2014, 00:00
Суть моего примера была в использовании сеттеров и геттеров для переменных приватного типа по принципу инкапсуляции.

Вот немного измененный вариант вашего примера с использованием сути которой я хотел донести.

public class User
{
	public static void main(String[] args)
	{
		User one = createUser("Timur", Role.USER);
		User two = createUser("L2CCCP", Role.ADMIN);

		System.out.println("У пользователя " + one.getName() + " права " + one.getRole());
		System.out.println("У пользователя " + two.getName() + " права " + two.getRole());
		System.out.println("------------------------------------");
		System.out.println("-----------Изменяем права-----------");
		System.out.println("------------------------------------");
		one.setRole(Role.ADMIN);
		two.setRole(Role.USER);
		System.out.println("У пользователя " + one.getName() + " права " + one.getRole());
		System.out.println("У пользователя " + two.getName() + " права " + two.getRole());
	}

	private String name;
	private Role role;

	private User(String name, Role role)
	{
		this.name = name;
		this.role = role;
	}

	public static User createUser(String name, Role role)
	{
		return new User(name, role);
	}

	public void setRole(Role role)
	{
		this.role = role;
	}

	public Role getRole()
	{
		return role;
	}

	public void setName(String name)
	{
		this.name = name;
	}

	public String getName()
	{
		return name;
	}

	public enum Role
	{
		USER,
		ADMIN
	}
}

Использовать данный код удобней как на мой взгляд.

А на счет дефолтной инициализации, дело не в книгах, а в привычке так как если Вы писали высоконагруженные приложения то должны знать что инициализацию нужно делать для избежания ошибок при изъятии данных от Объектов.

Надеюсь ответи
Timur 20 уровень
20 апреля 2014, 06:39
Использовать данный код удобней как на мой взгляд.

Да для простоты примера я не стал добавлять Getters, Setters, toString, Comparable и т.д. хотя можно было бы.

Но мое замечание было насчет читаймости вашего примера а не его инкапсуляции)

А на счет дефолтной инициализации, дело не в книгах, а в привычке так как если Вы писали высоконагруженные приложения то должны знать что инициализацию нужно делать для избежания ошибок при изъятии данных от Объектов.

А можно пруф? Так как в данном примере:

...
private boolean isAdmin = false; //Default is false
...


boolean — примитивный тип данных и Java гарантирует нам что при при изъятии там будет false если мы даже явно не инициализируем данную переменную. Какие ошибки вы имеете ввиду?
MikkiKliman 34 уровень, Санкт-Петербург
18 апреля 2014, 20:02
boolean is_admin;
// something
Boolean b = new Boolean( is_admin );
if( b == true) {
// something…
}
// something
Timur 20 уровень
18 апреля 2014, 19:20
public class User {
    private String name;
    private Role role;

    private User(String name, Role role) {
        this.name = name;
        this.role = role;
    }

    public static User createUser(String name, Role role) {
        return new User(name, role);
    }

    public boolean isRoleAdmin() {
        return role == Role.ADMIN;
    }

    public enum Role {USER, ADMIN}
}
KeLsTaR 24 уровень, Минск
22 апреля 2014, 01:11
Прямо без проверки корректности имени? На null проверить хотя бы.

И мне не понятен смысл приватного конструктора и вызов его же через отдельный публичный метод, просветите.
Timur 20 уровень
22 апреля 2014, 16:12
Прямо без проверки корректности имени? На null проверить хотя бы.
Да проверку на null стоило бы добавить. Насчет корректности тут уже зависит от требований.

if (name == null)
    throw new IllegalArgumentException("Some message ...");

Или можно также подключить Lombok

И мне не понятен смысл приватного конструктора и вызов его же через отдельный публичный метод, просветите.
Effective Java 2nd Edition, Item 1: Consider static factory methods instead of constructors
KeLsTaR 24 уровень, Минск
23 апреля 2014, 08:01
Мне кажется, ты не верно понял суть такого подхода с конструкторами. Там говорится о том, что статические методы удобнее конструкторов когда либо у тебя есть много одинаковых конструкторов, отличающихся только списком параметров(лучше оставить 1 конструктор и методы ссылать на него), либо ты не хочешь всегда возвращать новый объект (синглон там, или енамы), либо тебе надо возвращать объект подкласса (как в классе Calendar), либо тебе нужно как-то назвать конструктор, чтобы он имел более очевидный результат выполнения. В этом коде нету ничего, чему статический метод мог бы помочь, он у тебя просто переводит в конструктор все, но при этом есть большой минус — от такого юзера ты уже не сможешь наследоваться.
Timur 20 уровень
23 апреля 2014, 17:16
Ну обычно у юзеров несколько ролей например простые юзеры, модераторы, супермодераторы, тех. поддержка, админы и может понадобиться создать несколько методов createUser, createModerator, createSuperModerator, createAdmin… isRoleModerator,…
в свою очередь у каждого могут быть разные свойства. В зависимости от задачи ты дальше будешь решать как тебе изменить класс может ты решишь прописать свойства в Role enum может выделить для этого другой класс и т.д. и т.п.

Так как тут нет конкретной задачи я предложил свой вариант ты можешь улучшить его или написать свой. #comment16216 тут я уже писал что не стал включать геттеры и остальное.

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

А дальше ты читал?

The main disadvantage of providing only static factory methods is that classes without public or protected constructors cannot be subclassed.

Arguably this can be a blessing in disguise, as it encourages programmers to use composition instead of inheritance (Item 16).


www.artima.com/lejava/articles/designprinciples4.html
Item16: Favor composition over inheritance

ну если тебе прям так надо ты можешь изменить private на protected
max 30 уровень
18 апреля 2014, 16:54
// something…

))
terranum 28 уровень, Milan
18 апреля 2014, 17:00
// anything...
terranum 28 уровень, Milan
18 апреля 2014, 17:00
Да, я украл твою идею)
terranum 28 уровень, Milan
18 апреля 2014, 16:40
// something
if(;;) {
   // something...
}
// something
SergeyKandalintsev 32 уровень, Днепр
18 апреля 2014, 16:43
ШТОАААААА!!?
terranum 28 уровень, Milan
18 апреля 2014, 16:47
Это пример кода похуже)))
SergeyKandalintsev 32 уровень, Днепр
18 апреля 2014, 16:49
начнем с того что задача топика код улучшать, закончим тем что это совсем не код…
terranum 28 уровень, Milan
18 апреля 2014, 16:50
Никогда так не делайте!
terranum 28 уровень, Milan
18 апреля 2014, 16:50
ок, кончай
Sant9Iga 41 уровень
18 апреля 2014, 13:02
boolean is_admin;
// something

        if( is_admin) {
            // something...
        }
// something
Izuver 31 уровень
18 апреля 2014, 21:56
Мне кажется, что стоит также изменить имя boolean переменной на admin. Сдвигать if табулятором тоже не стоило. Поправьте, если не прав.
gram2005 30 уровень
18 апреля 2014, 22:31
Тогда уж лучше CamelCase: isAdmin