0

I have a class where I want to use an API, and I've got the api keys in a separate file in another directory.

The api key file is literally as simple as this:

$id = 'xxxxxxx';
$key = 'xxxxxxx';

This doesn't work:

include '/path/to/file-with-api-keys.php';

class MyApiClass {
    public function __construct($id, $token) {
        $this->client = new Client($id, $token);
    }
}

The code where I instantiate the class is in another php file I'm using to test, and it's extremely simple, just includes the class and then instantiates it:

include '/path/to/MyClass.php';

$result = new MyClass();
$result->myMethod();

echo $result;

The error I get is basically saying the 2 variables are null.

TWO QUESTIONS:

1) How can I access the value of the variables in my constructor? I've read elsewhere that using an include file directly in the method is bad practice, and also that using a global variable would be bad practice as well.

2) Somewhat related question, these files where I'm storing the api keys are in the same directory with my database connection details, but the directory is not outside the root. In this directory I have an .htaccess file with "Deny From All". Is this sufficient from a security standpoint, or should I do something else?

ok 3 questions...

3) Should I even bother keeping the api keys in separate files within this directory or just embed them into my class?

Hoping someone can give me best practices here. Thanks!

hyphen
  • 957
  • 1
  • 11
  • 31
  • Where's the code where you instantiate the class? And please provide and example of what's in the file containing the keys. Environment variables, or environment specific configs are pretty much the norm at the moment in my experience. – Jonnix Oct 23 '16 at 20:54
  • So, first off if `$id` and `$key` are required, then use `require` rather than `include`. Second, going be the provided code, as long as the classes are defined `new MyApiClass($id, $key)` should work fine and be passed into `Client`. – Jonnix Oct 23 '16 at 20:59
  • changed to require and got this error: Fatal error: require(): Failed opening required '/path/to/file-with-api-keys.php'; – hyphen Oct 23 '16 at 21:02
  • Then the file you're trying to add isn't where you've told PHP it is. Fix that first, otherwise you've no data to pass anyway. – Jonnix Oct 23 '16 at 21:04
  • Any reason why this wouldn't work? `require dirname(__DIR__) . '/path/to/file-with-api-keys.php';` ? If I echo that value it displays the correct location of the file, and I've also verified that the file exists in the directory. – hyphen Oct 23 '16 at 21:09
  • 1
    Your constructor expects 2 parameters to be passed along when instantiating the class. `$result = new MyClass();` will never work. It should be `$result = new MyClass("id","key");` – icecub Oct 23 '16 at 21:10
  • So then I should include the api keys file directly in my main script where I'm instantiating the class? The way I have it now they're just included in the class file. – hyphen Oct 23 '16 at 21:11
  • That, or give them default values. `public function __construct($myId = $id, $myToken = $token) {` – icecub Oct 23 '16 at 21:13
  • what about namespacing the API I'm using, should that happen wherever I instantiate the class as well? – hyphen Oct 23 '16 at 21:13

1 Answers1

1

  1. The constructor won't read the variables from file-with-api-keys.php automagically. You must specify them like this when instantiating the class: $result = new MyClass($id, $key);

  2. The best practice is to store all your PHP scripts outside the webroot directory except index.php (aka front controller).

  3. Yes, you should bother :) All configuration, API keys etc. should be stored in separate files, outside your classes code. You shouldn't mix these two things. If you're going to use some version control system like Git, then you're going to commit your classes code without any configuration details. The latter will be in your .gitignore file. If you ever work in a team of programmers, then every one of them is going to have his separate configuration files.

Piotr
  • 321
  • 2
  • 6
  • 1
    I agree with your answer to question 3, but the other 2, no. He could overload the class constructor and / or give default values to the parameters. Also, best practise is to store any PHP file with *sensative* information like database credentials outside of document root *or* a htaccess protected directory. Any other PHP files are perfectly fine within the document root. – icecub Oct 23 '16 at 21:20
  • This seems to have gotten me past the properties being accessed, but now I'm getting an error about instantiating the "Client" class in my constructor. Maybe I need to ask this in a separate SO question, but to help in my research, what is it called when you are instantiating another class from within a class? Is that even good practice? New to OOP. – hyphen Oct 23 '16 at 21:24
  • 1
    @hyphen Instantiating classes within classes is perfectly fine. The question is if it's really needed? Perhaps a class extension is a better solution depending on the corelation between the two classes. It might also be interesting to take a look at [Autoloading classes](http://php.net/manual/en/language.oop5.autoload.php) and [spl_autoload_register](http://php.net/manual/en/function.spl-autoload-register.php) – icecub Oct 23 '16 at 21:29
  • 1
    I'm already utilizing spl_autoload_register(), and the API I'm using has it's own autoload class. Perhaps extending the base class is the way to go. I'm going to dig into that and perhaps post another question if I can't get to the answer myself. Thanks! – hyphen Oct 23 '16 at 21:33