• ,

Говнокод #1

boolean is_admin;
// something
Boolean b = new Boolean( is_admin );
if( b.toString().length() == 4 ) {
   // something...
}
// something

29 комментариев

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

// something
if(;;) {
   // something...
}
// something
SergeyKandalintsev
ШТОАААААА!!?
terranum
Это пример кода похуже)))
SergeyKandalintsev
начнем с того что задача топика код улучшать, закончим тем что это совсем не код…
terranum
ок, кончай
terranum
Никогда так не делайте!
max
  • max
  • -1
  • Комментарий отредактирован 2014-04-18 16:54:21 пользователем max
// something…

))
terranum
// anything...
terranum
Да, я украл твою идею)
Timur

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
Прямо без проверки корректности имени? На null проверить хотя бы.

И мне не понятен смысл приватного конструктора и вызов его же через отдельный публичный метод, просветите.
Timur
  • Timur
  • 0
  • Комментарий отредактирован 2014-04-22 16:12:58 пользователем Timur
Прямо без проверки корректности имени? На 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
Мне кажется, ты не верно понял суть такого подхода с конструкторами. Там говорится о том, что статические методы удобнее конструкторов когда либо у тебя есть много одинаковых конструкторов, отличающихся только списком параметров(лучше оставить 1 конструктор и методы ссылать на него), либо ты не хочешь всегда возвращать новый объект (синглон там, или енамы), либо тебе надо возвращать объект подкласса (как в классе Calendar), либо тебе нужно как-то назвать конструктор, чтобы он имел более очевидный результат выполнения. В этом коде нету ничего, чему статический метод мог бы помочь, он у тебя просто переводит в конструктор все, но при этом есть большой минус — от такого юзера ты уже не сможешь наследоваться.
Timur
Ну обычно у юзеров несколько ролей например простые юзеры, модераторы, супермодераторы, тех. поддержка, админы и может понадобиться создать несколько методов 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
MikkiKliman
  • MikkiKliman
  • -2
  • Комментарий отредактирован 2014-04-18 20:02:01 пользователем MikkiKliman
boolean is_admin;
// something
Boolean b = new Boolean( is_admin );
if( b == true) {
// something…
}
// something
L2CCCP
  • L2CCCP
  • -2
  • Комментарий отредактирован 2014-04-19 02:21:51 пользователем L2CCCP
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
Я поставил минус так как если бы я использовал твой класс то в итоге было бы что-то типо этого:

...
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
Если Вы слышали о таком принципе ООП как инкапсуляция Вы поймете мой пример :)
Groomsh
А зачем эта строка, если не секрет?)
В смысле — зачем явное указание, учитывая, что есть дефолтная инициализация переменных?)
private boolean isAdmin = false; //Default is false
Timur
Наверно из-за того что во многих книгах советуют явно в некоторых случаях инициализировать переменные для лучшей читабельности например
счетчик советуют всегда инициализировать явно.
Timur
  • Timur
  • 0
  • Комментарий отредактирован 2014-04-19 19:21:46 пользователем Timur
Я слышал и прочитал не одну книгу и смотрел много всяких исходников поэтому я и спросил) Тут вопрос насчет читабельности а не инкапсуляции мне кажется если сделать хотя бы из getAdmin() -> isAdmin() и подумать еще над именем класса читаймость бы увеличилась.

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

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


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
Использовать данный код удобней как на мой взгляд.

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

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

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

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


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


boolean — примитивный тип данных и Java гарантирует нам что при при изъятии там будет false если мы даже явно не инициализируем данную переменную. Какие ошибки вы имеете ввиду?
Predict_it
  • Predict_it
  • -1
  • Комментарий отредактирован 2014-04-20 02:29:33 пользователем Predict_it
int var;
int var2;
int var3;
int var4;

int var5 = var + var2 + var3 + var4;
alexnjc
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
IvanDurov
return 5 < age ? false : true

Правильно:
return age <= 5
Только зарегистрированные и авторизованные пользователи могут оставлять комментарии.