New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Object 클래스의 이름 충돌로 인한 클래스 이름 변경 #2181

Closed
bnu opened this Issue Nov 27, 2017 · 7 comments

Comments

@bnu
Member

bnu commented Nov 27, 2017

PHP 7.2에서 Object 클래스를 사용할 수 없음.
PHP 7부터 예약어로 지정됐고, 최근 PHP 7.2에서 변경으로 인해 사용이 제한 됨.
PHP 7.2는 현재 RC6 단계이지만 이 변경 사항은 정식버전에도 반영될 것으로 보임

XE는 이러한 PHP 7.2의 변경사항에 맞춰 Object 클래스의 이름을 BaseObject로 변경합니다.
이로 인한 호환성 문제를 피하기 위해 PHP 7.2 미만에서는 여전히 Object 클래스를 사용할 수 있습니다
(class_alias()를 이용해 Object클래스를 유지합니다)

이는 PHP 7.2의 변경으로 인해 XE가 변경을 따라갈 수 밖에 없는 부분입니다.
return new Object(...)과 같은 코드를 사용한 모듈, 애드온, 위젯 등은 이 변경사항을 반영하지 못하면 PHP 7.2 이상에서 동작할 수 없습니다. 다음과 같이 변경 사항을 적용해야 PHP 7.2 미만과 이상에서도 동작할 수 있습니다.

써드파티 모듈 등에서 return new Object(-1, 'error message')처럼 사용하고 있다면 다음과 같이 수정해야 합니다.

return new Object(-1, 'errmsg');
$output = new Object();
// 이와 같이 사용했다면 아래와 같이 변경

return class_exists('BaseObject') ? new BaseObject(-1, 'errmsg') : new Object(-1, 'errmsg');
$output = class_exists('BaseObject') ? new BaseObject() : new Object();

이름이 바뀐 BaseObjectObject 클래스를 모두 지원할 수 있도록 합니다.

@kijin 님께서 제시해주신 코드이며, 동작에 문제가 없는 것 까지 확인해주셨습니다.
다만, PHP 7.2의 정식 버전이 배포되기 이전이므로 PHP 7.2 정식 버전 및 이후의 변경에 따라 이 방법이 유효하지 않을 수도 있습니다.

@bnu bnu self-assigned this Nov 27, 2017

@bnu bnu changed the title from Object 클랙스의 이름 충돌 to Object 클래스의 이름 충돌로 인한 클래스 이름 변경 Nov 27, 2017

@bnu bnu added this to the 1.9.0 milestone Nov 27, 2017

@bnu bnu removed the type/enhancement label Nov 27, 2017

@bnu bnu added this to 해결 중 in 이슈 진행 상황 Nov 27, 2017

@bjrambo

This comment has been minimized.

Show comment
Hide comment
@bjrambo

bjrambo Nov 27, 2017

Contributor

return $this->stop('text'); 을 사용하는 경우도 있던데... 이부분은 어떻게 처리하실 예정인가요?

Contributor

bjrambo commented Nov 27, 2017

return $this->stop('text'); 을 사용하는 경우도 있던데... 이부분은 어떻게 처리하실 예정인가요?

kijin added a commit to rhymix/rhymix that referenced this issue Nov 27, 2017

Allow adding error message and sprintf() variables using setError()
xpressengine/xe-core#2181 적용시 에러 반환 문법을 단순화하기 위한 조치

기존 방식: return new Object(-1, '에러메시지');
XE 제안 방식: return class_exists('BaseObject') ? new BaseObject(-1, '에러메시지') : new Object('에러메시지');
라이믹스 방식: return $this->setError('에러메시지');

기존의 setError() 메소드가 에러 코드만 받을 수 있어서 호환성 보장에 도움이 안 되므로
에러 코드와 에러 메시지를 동시에 넣을 수 있도록 개선하고,
에러 코드를 넣지 않고 에러 메시지만 지정해도 자동으로 -1 에러 코드가 들어가도록 하였음.
(첫 번째 인자가 정수인지 아닌지에 따라 판단함.)

setError(), setMessage(), setMessageType() 등 기존에 무의미한 반환값을 가지던 메소스들 모두
$this를 반환하도록 함으로써 액션이나 트리거 등의 반환값으로 유효하도록 하고,
원할 경우 method chaining까지 사용할 수 있음.

또한 에러메시지에 변수를 넣어야 할 경우
return new Object(-1, sprintf(Context::getLang('error_msg'), $var1, $var2));
이렇게 복잡해지는 문제도 해결하기 위해
setError()에 추가로 넣은 인자는 모두 자동으로 sprintf() 처리를 거치도록 함.
예: return $this->setError('error_msg', $var1, $var2);

즉, 아래와 같은 호출 형태가 모두 유효함.

  - $this->setError(-1);
  - $this->setError(-1, 'error_msg');
  - $this->setError(-1, 'error_msg', $var1, $var2);
  - $this->setError('error_msg');
  - $this->setError('error_msg', $var1, $var2);

단, 이 커밋 이후 신규 작성하는 코어 클래스나 서드파티 자료에서만 사용할 수 있음.
기존 버전과의 호환성을 유지하기를 원하는 서드파티 자료는 XE에서 제안한 삼항식을 사용해야 함.
@kijin

This comment has been minimized.

Show comment
Hide comment
@kijin

kijin Nov 27, 2017

Contributor

서드파티 자료 제작자 중 수정하실 부분이 많다면 아래와 같이 편리하게 처리할 수도 있습니다.

우선 모듈명.class.php에 정의된 모듈 기본 클래스에 메소드 하나를 추가합니다.

public function makeObject($code, $msg)
{
    return class_exists('BaseObject') ? new BaseObject($code, $msg) : new Object($code, $msg);
}

그리고 이 클래스를 상속받는 다른 모든 클래스(컨트롤러, 모델, 뷰 등)에서

return new Object(-1, 'errmsg');

라고 되어 있던 부분을 모두

return $this->makeObject(-1, 'errmsg');

라고 바꾸시면 그나마 깔끔하게 호환성을 유지할 수 있습니다.

서드파티 모듈 내에서 이런 함수는 얼마든지 자유롭게 만들어 쓰셔도 됩니다.

Contributor

kijin commented Nov 27, 2017

서드파티 자료 제작자 중 수정하실 부분이 많다면 아래와 같이 편리하게 처리할 수도 있습니다.

우선 모듈명.class.php에 정의된 모듈 기본 클래스에 메소드 하나를 추가합니다.

public function makeObject($code, $msg)
{
    return class_exists('BaseObject') ? new BaseObject($code, $msg) : new Object($code, $msg);
}

그리고 이 클래스를 상속받는 다른 모든 클래스(컨트롤러, 모델, 뷰 등)에서

return new Object(-1, 'errmsg');

라고 되어 있던 부분을 모두

return $this->makeObject(-1, 'errmsg');

라고 바꾸시면 그나마 깔끔하게 호환성을 유지할 수 있습니다.

서드파티 모듈 내에서 이런 함수는 얼마든지 자유롭게 만들어 쓰셔도 됩니다.

@howtoxe

This comment has been minimized.

Show comment
Hide comment
@howtoxe

howtoxe Nov 27, 2017

Contributor

@kijin 해당 함수를 XE 코어 차원에서 지원하는 편이 나을 것 같습니다.

Contributor

howtoxe commented Nov 27, 2017

@kijin 해당 함수를 XE 코어 차원에서 지원하는 편이 나을 것 같습니다.

@bjrambo

This comment has been minimized.

Show comment
Hide comment
@bjrambo

bjrambo Nov 27, 2017

Contributor

@howtoxe 서드파티에서 XE코어 함수라고 썻다가 이전 버전에서 실행하지 못하는 경우떄문에 모듈 클래스로 만들라고 하는겁니다..ㅡ.ㅡ;;ㅋㅋㅋ

Contributor

bjrambo commented Nov 27, 2017

@howtoxe 서드파티에서 XE코어 함수라고 썻다가 이전 버전에서 실행하지 못하는 경우떄문에 모듈 클래스로 만들라고 하는겁니다..ㅡ.ㅡ;;ㅋㅋㅋ

@kijin

This comment has been minimized.

Show comment
Hide comment
@kijin

kijin Nov 27, 2017

Contributor

@howtoxe XE 구버전과 신버전을 모두 지원해야 하는 서드파티 자료 개발자 입장에서는 이제 와서 함수를 새로 만든다 해도 도움이 되지 않습니다. XE 구버전에는 해당 함수가 없었으니까요... 여전히 function_exists 등을 사용해서 삼항식으로 처리해야 하겠지요.

Contributor

kijin commented Nov 27, 2017

@howtoxe XE 구버전과 신버전을 모두 지원해야 하는 서드파티 자료 개발자 입장에서는 이제 와서 함수를 새로 만든다 해도 도움이 되지 않습니다. XE 구버전에는 해당 함수가 없었으니까요... 여전히 function_exists 등을 사용해서 삼항식으로 처리해야 하겠지요.

@howtoxe

This comment has been minimized.

Show comment
Hide comment
@howtoxe

howtoxe Nov 27, 2017

Contributor

@kijin @bjrambo 아 그렇겠네요. 이 이슈보는 순간 왠지모르게 귀차니즘이 몰려와서 아무 생각없이 달아버렸네요 ㅋㅋ

Contributor

howtoxe commented Nov 27, 2017

@kijin @bjrambo 아 그렇겠네요. 이 이슈보는 순간 왠지모르게 귀차니즘이 몰려와서 아무 생각없이 달아버렸네요 ㅋㅋ

bnu added a commit that referenced this issue Nov 28, 2017

@bnu bnu referenced this issue Nov 28, 2017

Closed

PHP 7.2 지원 #34

@bnu

This comment has been minimized.

Show comment
Hide comment
@bnu

bnu Nov 29, 2017

Member

@kijin 님이 가이드 하신 것처럼 krzip 모듈에 적용했습니다.
단순한 치환이지만 아래 커밋을 참고해주세요.

xpressengine/xe-module-krzip@8516c05

Member

bnu commented Nov 29, 2017

@kijin 님이 가이드 하신 것처럼 krzip 모듈에 적용했습니다.
단순한 치환이지만 아래 커밋을 참고해주세요.

xpressengine/xe-module-krzip@8516c05

@bnu bnu closed this Nov 29, 2017

이슈 진행 상황 automation moved this from 해결 중 to 이슈 종료 Nov 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment