Allow extensions to override methods and the property

[antonioribeiro]

No description provided.

[vlucas]

There's really no reason not to merge this. I do agree with the open to modifications principle. Thoughts @kelvinj ?

[kelvinj]

Open to modifications is fine, but then that doesn't have to come through inheritance.

In fact, as this library is more of a functional library (that happens to be bunded as a class), I'd be less inclined to support inheritance… if it were implemented as a bunch of namespaced functions, you wouldn't have inheritance as an option.

@antonioribeiro what kind of of extension are you thinking about?

[antonioribeiro]

I was was in the process to extend it to treat booleans as I was doing while loading PHP arrays to the environment. So I extended your sanitiseVariableValue to transform the values:

protected static function sanitiseVariableValue($value)
{
    $value = parent::sanitiseVariableValue($value);

    return static::toString($value);
}

This toString transforms a PHP false to a string (false), and, of course, I would have to use an internal method to convert it back. Not really a problem to be solved by PHP dotenv, because it's a PHP problem, putenv saves everything as strings and 'false' becomes true in PHP conditionals.

But... after sendind that pull request I realized I could not solve that problem with PHP dotenv too, because it doesn't use PHP arrays, but ini files, which stores everything as strings as well. So now to use PHP dotenv I'll have to use conventions and not real data types. :)

Anyway, I still think you should give room for people to (at least) extend that sanitiseVariableValue and transform values before they go to the environment.

[kelvinj]

I agree about exposing points in the process to allow certain modification, I just feel uneasy about doing that through inheritance… I think it opens a can of worms.

You could extend using a callback to cast the value as a boolean but, as you've mentioned, putenv only takes a string. This would leave your $_ENV value as a bool and your getenv() value as a string. I'm not sure that a good situation.

Out of interest, after using Dotenv to load the .env file, how are you then accessing the environment variable? Using getenv()?

ps: when needing booleans, I just cast a boolean on-the-fly, e.g. (getenv('APP_DEBUG') == 'true').
pps: it's not an ini file, more like a series of environment setters you'd find in a .bashrc file.

[antonioribeiro]

I tried to use $_ENV, but they are all strings too, probably due to the fact that you load strings and I couldn't find code converting them to php data types. Those are the debug windows showing the file loaded by dotenv and $_ENV after the setEnvironmentVariable loop:

There are many other datatypes tests becase my code is taking from getenv() and converting them all. So I can:

env('APP_FLOAT') === 3.141592
env('APP_INT') === 8192
env('APP_NUM_STRING') === '8192'
env('APP_EMPTY') === ''
env('APP_YES') === true
env('APP_NO') === false
env('APP_NULL') === null
env('APP_TRUE') === true
env('APP_FALSE') === false

[vlucas]

@antonioribeiro if you store all your configuration in the environment, this is always going to be an issue. Environment variables/shell variables don't really have any type, and will always come through as simple strings when you get them in your application, just like POST and query string data. I am not sure your use case here, but the Dotenv solution is primarily for sensitive configuration information like API keys and passwords, etc. It's not a total config solution or replacement.

That being said, I don't see any harm at all in making things protected vs. private. In fact, I tend to want to allow inheritance, because it is a common and easy way to hack on and modify existing code. Since this library is currently only a handful of static method calls, there is really no way to modify it in a better way with composition or even wrap it with another object. That will change in v2 (Dotenv will be an instance instead of static), but until then, this is what we have.

[antonioribeiro]

Yes I use them basically for sensitive data, but also for things I cannot risk having wrong in the production environment. An example is the application in debug mode. I must have DEBUG_MODE=false in production, and if the variable is not set I want the application to crash, it's better to see it awfully crashing right after deploy than me going to sleep thinking everything is good. I also might need to enable it in staging, to debug something happening at that particular server, this way I don't have to touch actual source code (in Laravel it would be the app/config/app.php file) to temporarily put my app in debug mode. Yes, I could easily set that in the configuration files, but I would have to add logic to them, to compare the environment name, and I don't really like logic in configuration files, other than a 'debug' => env('DEBUG_MODE'). Not sure if I'm doing it wrong or not, but this is working fine for me.

Thanks for merging it.

And very good article, by the way. ;)