-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add more testing and fix some errors #25
base: master
Are you sure you want to change the base?
Changes from all commits
fa42d35
e184392
748cc0e
346d209
3d87b0c
3b8955b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,15 @@ | ||
language: php | ||
dist: precise | ||
php: | ||
- "7.1" | ||
- "7.0" | ||
- "5.6" | ||
- "5.5" | ||
- "5.4" | ||
- "5.3" | ||
before_script: | ||
- composer self-update | ||
- composer install --prefer-source --no-interaction --dev | ||
- 7.1 | ||
- 7.0 | ||
- 5.6 | ||
- 5.5 | ||
- 5.4 | ||
- 5.3 | ||
install: | ||
- composer install | ||
script: | ||
- phpunit --coverage-clover build/logs/clover.xml | ||
after_success: | ||
- travis_retry vendor/bin/coveralls -v |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,4 +55,17 @@ public function testFactorialVariousValidFactorials() | |
$f = new Factorial(5); | ||
$this->assertEquals(120, $f->result); | ||
} | ||
|
||
public function testGetShouldBeNull() | ||
{ | ||
$f = new Factorial(2); | ||
$this->assertNull($f->__get(null)); | ||
} | ||
|
||
public function testToString() | ||
{ | ||
$f = new Factorial(2); | ||
$this->assertInternalType('string', $f->__toString()); | ||
$this->assertSame('2', $f->__toString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ... or just |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,12 +63,35 @@ public function testIfRealIsIntegerOrNotShouldSuccess() | |
$this->assertTrue($i->isInteger()); | ||
} | ||
|
||
|
||
public function testStringContextShouldSuccess() | ||
{ | ||
$i = new Real(2.56); | ||
$d = new Real(0.56); | ||
$this->assertEquals('2.56', "$i"); | ||
$this->assertEquals('0.56', "$d"); | ||
} | ||
|
||
public function testGetShouldBePI() | ||
{ | ||
$real = new Real(2.56); | ||
$this->assertEquals(M_PI, $real->__get('pi')->__toString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum… I dont know, this is strange to get pi, e, and so on from an instance. Confusing. But, we can do that: $real = new Real(Real::PI);
$this->assertEquals(M_PI, "$real"); |
||
} | ||
|
||
public function testGetShouldBeE() | ||
{ | ||
$real = new Real(2.56); | ||
$this->assertEquals(M_E, $real->__get('e')->__toString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same idea as in fa42d35#r150423847 |
||
} | ||
|
||
public function testGetShouldBeEuler() | ||
{ | ||
$real = new Real(2.56); | ||
$this->assertEquals(M_EULER, $real->__get('euler')->__toString()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as fa42d35#r150423847 |
||
} | ||
|
||
public function testGetShouldBeNull() | ||
{ | ||
$real = new Real(2.56); | ||
$this->assertNull($real->__get('no')); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
This break the possibility to have
$object->rho
, or$object->theta
or$object->r
or$object->i
direct read access.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think directly return
$name
is appropriate.If set the original code
$this->$name
, the variable will return null.Because this variable is not defined in this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about declare a
private $name
and change$this->$name
to$this->name
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean, but it is confusing, really.
But this class should be change in futur I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want keep magic getter to get this values, less confusing I think.