I think it would be better to publish the individual structures as standalone files without dependencies.
Then I might use one of them in the future.
But pulling in thousands of lines of code by someone from the internet is nothing I would ever do. Because I cannot review it all to be sure it is not malicious. Reviewing a single file without dependencies is something I might do.
Maybe I missed something, but you can clearly go into each file in the src folder. I took a quick glance and things seem to be fairly small and separated. You can always just take the class/method you want without taking all the files, or package.
Not associated with the project, but confused as to the follow-up threads.
This concept of reviewing every line of OSS and copy / pasting the ones you agree with is really holding PHP dev back. Embracing dependency management is an important way to focus your codebase on your application code and is just a modern practice in all languages (https://12factor.net/dependencies). This is part of why JS (particularly node / isomorphic) is continuing to take over previously PHP marketshare. The actual (vs handwavey) security risks are low especially for popular projects.
In this case, if the datastructure or algorithm were useful to your project, you could:
1. Not use the algorithm / data structure at all, resulting in worse performance.
2. Hand roll your own version which is more likely to have improper implementation issues than an OSS version, likely resulting in performance or security issues and wasting your time.
3. Use the OSS version which is likely to have bugs / errors / security issues already solved.
This concept of not reviewing any libraries you rely on, and installing them from a third party, mutable source is really holding nodejs back.
Embracing reliable development and deployment practices would prevent you from being the butt of the joke next time someone pulls a leftpad, or what have you.
Sure, so you can represent the other far extreme. I certainly advocate for review, but we should be practical. No one reads all the code down to the proc to ensure nothing malicious is happening, and the average software dev wouldn't be able to understand something malicious at that level anyway. I could have used a more hyperbolic example than a practical one (have you read all the PHP source for the version you're on? Have you read all your PECL libraries? Those ones are okay but OSS in PHP is not? Why? Just because it's harder to read C, right?). Pretending we do review everything is disingenuous -- but we should do what we can. Review doesn't have to be reading lines of code, but it could be ensuring it comes from popular and trusted sources (part of my earlier comment).
It should be noted that your example was not bad source, so rigorously reviewing source code would not have helped. It was an unpublish event which was unexpected but is now differently handled by the package managers + registries.
Your example about php source, or any of the bundled extensions would be a valid point if they were maintained by one or two single developers.
As you say, it’s impossible to review everything.
But how many projects have been caught out by sudden changes to previously working dependencies? Either revoked code, malware inclusion, breaking changes, etc.
If your dependencies are versioned and go through a review like internal changes, it’s much easier to spot changes.
I agree that a trusted source is invaluable- my point is that (a) npm hasn’t shown itself to be that and (b) the “externalise all the things” approach pushed so heavily by the nodejs community means it’s not just your dependencies you need to worry about - it’s the crab-grass like tree of nested dependencies.
there is much more than just providing OS source code. I personally think that single files are going to be overkill and the "review" will not happen because it will be too much.
On the other side, the source code is on GitHub. You (and many other developer) are welcome to review. GitHub would not allow malicious code to be hosted on their platform. And in the end, it is the same process as you would review single files..
Further, there is also the aspect of dependencies: the power of OS software provides the ability to use other OS libraries. This means that I do not need to reinvent the wheel and use existing solutions. Providing single files would require that you make a bunch of 'include' statements before you can run my code. This is pain..
And finally, I - as the creator - guarantee that PHPAlgorithms does not do malicious things :)
The problem is that no one reviews what goes in the package registries. Unless someone spots the security issue it goes unnoticed and unfixed. Particularly true with new and not very popular packages.
To be fair to the author though this package won't take more than a couple of hours review quickly for potential back doors and if you just want one data structure it's at most in 3 files.
It makes more sense you keep only the files you need, I do not understand how exactly the author should bundle this OOP using algorithms in 1 source file that will satisfy everyone requirements and at the same time won't contain some code you won't use.
I just had a quick look, some observations if you don't mind,
- use a good linter and fix all warnings you get from it if possible
- use code formatting so all your code has the same formatting
- you put the isDead function in the Piece class and pass the board object, IMO it makes more sense to put it in the Board class or even better make a Game/Puzzle class , then you can reuse the Board and Piece unchanged when you implement different rules but the board and pieces are the same.
Hey this is fantastic! I don't care about the job interview part, but having the ability to see some of these algorithms implemented in my daily workhorse may help me improve as a programmer. So thanks!
Then I might use one of them in the future.
But pulling in thousands of lines of code by someone from the internet is nothing I would ever do. Because I cannot review it all to be sure it is not malicious. Reviewing a single file without dependencies is something I might do.