Skip to content

change getDouble add constructor args - #130

Merged
kenjis merged 4 commits into
kenjis:masterfrom
ytetsuro:patch-1
Nov 6, 2016
Merged

change getDouble add constructor args#130
kenjis merged 4 commits into
kenjis:masterfrom
ytetsuro:patch-1

Conversation

@ytetsuro

@ytetsuro ytetsuro commented Jul 12, 2016

Copy link
Copy Markdown
Contributor

If i want to create a " Mock " on "SplFileObject" this feature was thought that want .

<?php

public function test_parse_csv()
{
    // disable constructor
    $spl_mock = $this->getDobule('SplFileObject', [
         'current' => ['foo', 'bar'],
         'next'      => NULL
    ]);
}

LogicException: The parent constructor was not called: the object is in an invalid state

<?php

public function test_parse_csv()
{
    // enable constructor
    $spl_mock = $this->getDobule('SplFileObject', [
         'current' => ['foo', 'bar'],
         'next'      => NULL
    ], true);
}

_RuntimeException: SplFileObject::_construct() expects at least 1 parameter, 0 given

Therefore , I want to describe as follows .

<?php

public function test_parse_csv()
{
    // set constructor args
    $spl_mock = $this->getDobule(
        'SplFileObject',
        [
            'current' => ['foo', 'bar'],
            'next'    => NULL
        ],
        ['php://memory']
    );
}

Change to be able to specify the "array" in the third argument of the "getDouble method"
@coveralls

coveralls commented Jul 12, 2016

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.08%) to 72.526% when pulling 9bc3dbc on ytetsuro:patch-1 into b1ea0eb on kenjis:master.

@tianhe1986

tianhe1986 commented Jul 13, 2016

Copy link
Copy Markdown

@ytetsuro Maybe you should change "getDouble" like this:

<?php
public function getDouble($classname, $params, $enable_constructor = false, $constructor_args = [])

and then describe as follows

<?php

public function test_parse_csv()
{
    // set constructor args
    $spl_mock = $this->getDobule('SplFileObject', [
         'current' => ['foo', 'bar'],
         'next'      => NULL
    ], true, ['php://memory']);
}

@kenjis

kenjis commented Jul 13, 2016

Copy link
Copy Markdown
Owner

@tianhe1986 Yeah, it is better. Because $enable_constructor in this PR is difficult to understand what it is.

But do we really need this? We don't have to use getDouble() in every case.
If this use case is very rare, we don't have to have it in getDouble().

@coveralls

coveralls commented Aug 5, 2016

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 72.885% when pulling a3319b2 on ytetsuro:patch-1 into b1ea0eb on kenjis:master.

@coveralls

coveralls commented Aug 5, 2016

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 72.885% when pulling a3319b2 on ytetsuro:patch-1 into b1ea0eb on kenjis:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.3%) to 72.885% when pulling d06065b on ytetsuro:patch-1 into b1ea0eb on kenjis:master.

@ytetsuro

Copy link
Copy Markdown
Contributor Author

@tianhe1986 Yeah, it is better. Because $enable_constructor in this PR is difficult to understand what it is.

I'm sorry.
me too.
I felt that I do not even understand while keep writing your own .

But do we really need this? We don't have to use getDouble() in every case.
If this use case is very rare, we don't have to have it in getDouble().

I think this is very rare cases.
But than may have the versatility if there is a setting item of constructor.

@kenjis
kenjis merged commit d06065b into kenjis:master Nov 6, 2016
@ytetsuro
ytetsuro deleted the patch-1 branch February 9, 2017 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants