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

NFR: Get one element of the array with config->get() #12221

Closed
xerron opened this Issue Sep 14, 2016 · 23 comments

Comments

Projects
None yet
9 participants
@xerron

xerron commented Sep 14, 2016

The good from the get function is that you can set a default value.

now:

$adapter = $this->config->database->adapter ? $this->config->database->adapter : 'Mysql' ;

This is better:

$adapter = $this->config->get('database.adapter', 'Mysql');
@mzf

This comment has been minimized.

Show comment
Hide comment
@mzf

mzf Sep 15, 2016

Dot notation is very good idea

mzf commented Sep 15, 2016

Dot notation is very good idea

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Sep 15, 2016

Member

I think will be better:

$adapter = $this->config->path('application.database.adapter', 'Mysql');
$adapter = $this->config->path('application/database/adapter', 'Mysql', '/');
Member

sergeyklay commented Sep 15, 2016

I think will be better:

$adapter = $this->config->path('application.database.adapter', 'Mysql');
$adapter = $this->config->path('application/database/adapter', 'Mysql', '/');
@mzf

This comment has been minimized.

Show comment
Hide comment
@mzf

mzf Sep 15, 2016

Ye. Fine option

mzf commented Sep 15, 2016

Ye. Fine option

@xerron

This comment has been minimized.

Show comment
Hide comment
@xerron

xerron Sep 16, 2016

it's a little confusing use 'path', for me is better 'get'

xerron commented Sep 16, 2016

it's a little confusing use 'path', for me is better 'get'

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Sep 16, 2016

Member

But what if for some weird reason someone already have named section with dot ?

Member

Jurigag commented Sep 16, 2016

But what if for some weird reason someone already have named section with dot ?

@xerron

This comment has been minimized.

Show comment
Hide comment
@xerron

xerron Sep 16, 2016

@Jurigag you mean this?

<?php
return new \Phalcon\Config([
    'database' => [
        'adapter' => 'Postgresql',
        'host' => 'localhost',
        'username' => 'xerron',
        'password' => 'root',
        'dbname' => 'db_uwu'
    ],
   // ...code ...
    ' section.with.dot' =>  'mmm...'
]

good question. 👍

xerron commented Sep 16, 2016

@Jurigag you mean this?

<?php
return new \Phalcon\Config([
    'database' => [
        'adapter' => 'Postgresql',
        'host' => 'localhost',
        'username' => 'xerron',
        'password' => 'root',
        'dbname' => 'db_uwu'
    ],
   // ...code ...
    ' section.with.dot' =>  'mmm...'
]

good question. 👍

@xerron

This comment has been minimized.

Show comment
Hide comment
@michanismus

This comment has been minimized.

Show comment
Hide comment
@michanismus

michanismus Sep 18, 2016

Contributor

I need this so bad! 100x thumbs up

Contributor

michanismus commented Sep 18, 2016

I need this so bad! 100x thumbs up

@sergeyklay sergeyklay added this to the 3.1.0 milestone Sep 18, 2016

@sergeyklay sergeyklay self-assigned this Sep 18, 2016

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Sep 18, 2016

Member

Will be implemented in 3.1.0

Member

sergeyklay commented Sep 18, 2016

Will be implemented in 3.1.0

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Sep 18, 2016

Member

So implement it in other places too, as far as i know people wanted this in forms too and somewhere else.

Member

Jurigag commented Sep 18, 2016

So implement it in other places too, as far as i know people wanted this in forms too and somewhere else.

@xerron

This comment has been minimized.

Show comment
Hide comment
@xerron

xerron Sep 21, 2016

In PHP7 default values is most readable:

$adapter =  $this->config->get('database.adapter') ?? 'Mysql';

xerron commented Sep 21, 2016

In PHP7 default values is most readable:

$adapter =  $this->config->get('database.adapter') ?? 'Mysql';
@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Sep 21, 2016

Member

but it's longer and we support not only PHP7

Member

sergeyklay commented Sep 21, 2016

but it's longer and we support not only PHP7

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Sep 21, 2016

Member

Just add option to just call $this->config->path('application.database.adapter') without second argument, and you can use default value like php7 with null coalescing operator.

Member

Jurigag commented Sep 21, 2016

Just add option to just call $this->config->path('application.database.adapter') without second argument, and you can use default value like php7 with null coalescing operator.

@xerron

This comment has been minimized.

Show comment
Hide comment
@xerron

xerron Sep 22, 2016

Well, this is good:

$adapter = $this->config->path('application/database/adapter', '/');

xerron commented Sep 22, 2016

Well, this is good:

$adapter = $this->config->path('application/database/adapter', '/');
@daison12006013

This comment has been minimized.

Show comment
Hide comment

This is cool

@ricksanchez

This comment has been minimized.

Show comment
Hide comment
@ricksanchez

ricksanchez Oct 4, 2016

@sergeyklay Actually what @xerron needs, is only:
$adapter = $this->config->database->adapter ?: 'Mysql' ;
Not PHP7 ?? required for this, just the classic Elvis operator ?:

@sergeyklay Actually what @xerron needs, is only:
$adapter = $this->config->database->adapter ?: 'Mysql' ;
Not PHP7 ?? required for this, just the classic Elvis operator ?:

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Oct 4, 2016

Member

@udarkness Related to #11157

Member

sergeyklay commented Oct 4, 2016

@udarkness Related to #11157

@Yurist-85

This comment has been minimized.

Show comment
Hide comment
@Yurist-85

Yurist-85 Dec 6, 2016

After read all above I think, the best way is something like this:

$config = new Config([ ... ]);
Use '.' [dot] notation by default but allowed to reset it if needed.
$config->setPathSeparator('/');

Use 3rd param to reset separator per request:
$val = $config->get('some.other.path.to_the.val', null, '.');

Return null if not exists (it's current behaviour I guess):
$val = $config->get('some/path/to/nonexistantval'); // null

Three ways to get default value:

$val = $config->get('some/path/to/nonexistantval', 'default_value');
$val = $config->get('some/path/to/nonexistantval') ?: 'default_value'; // 5.6
$val = $config->get('some/path/to/nonexistantval') ?? 'default_value'; // 7

And I'm absolutely sure that ->get() is much more clear and obvious than ->path() .

After read all above I think, the best way is something like this:

$config = new Config([ ... ]);
Use '.' [dot] notation by default but allowed to reset it if needed.
$config->setPathSeparator('/');

Use 3rd param to reset separator per request:
$val = $config->get('some.other.path.to_the.val', null, '.');

Return null if not exists (it's current behaviour I guess):
$val = $config->get('some/path/to/nonexistantval'); // null

Three ways to get default value:

$val = $config->get('some/path/to/nonexistantval', 'default_value');
$val = $config->get('some/path/to/nonexistantval') ?: 'default_value'; // 5.6
$val = $config->get('some/path/to/nonexistantval') ?? 'default_value'; // 7

And I'm absolutely sure that ->get() is much more clear and obvious than ->path() .

@fenixphp

This comment has been minimized.

Show comment
Hide comment
@fenixphp

fenixphp Feb 28, 2017

get($key, $default = null)

get('Dashboard/Main.plugins', [])

Up to the first dot, the path to the file. Dashboard/Main
Dashboard/Main.json or Dashboard/Main.php depending on the adapter.

After first dot using as array dot notation.

Something like this?

get($key, $default = null)

get('Dashboard/Main.plugins', [])

Up to the first dot, the path to the file. Dashboard/Main
Dashboard/Main.json or Dashboard/Main.php depending on the adapter.

After first dot using as array dot notation.

Something like this?

@Jurigag

This comment has been minimized.

Show comment
Hide comment
@Jurigag

Jurigag Feb 28, 2017

Member

What? But config adapter itself accept file path, so no, it can't be like this.

Member

Jurigag commented Feb 28, 2017

What? But config adapter itself accept file path, so no, it can't be like this.

@sergeyklay sergeyklay modified the milestones: 3.1.0, 3.2.0 Mar 2, 2017

@fenixphp

This comment has been minimized.

Show comment
Hide comment
@fenixphp

fenixphp Mar 2, 2017

А если он будет принимать путь к папке с конфигами, и получится ленивая загрузка конфигураций только там где это нужно и со значениями по умолчанию?


And if he will take the path to the configuration file, and you get lazy loading configurations only where it is needed and with default values?


Sorry translation may be inaccurate.

fenixphp commented Mar 2, 2017

А если он будет принимать путь к папке с конфигами, и получится ленивая загрузка конфигураций только там где это нужно и со значениями по умолчанию?


And if he will take the path to the configuration file, and you get lazy loading configurations only where it is needed and with default values?


Sorry translation may be inaccurate.

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Apr 6, 2017

Member

Implemented in the 3.2.x branch

Member

sergeyklay commented Apr 6, 2017

Implemented in the 3.2.x branch

@sergeyklay sergeyklay closed this Apr 6, 2017

@sergeyklay

This comment has been minimized.

Show comment
Hide comment
@sergeyklay

sergeyklay Apr 6, 2017

Member

@fenixphp This should be a separated NFR

Member

sergeyklay commented Apr 6, 2017

@fenixphp This should be a separated NFR

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