Can you please review my code

New to Julia
1

Hi everyone, I’m actually a 17 years old young Cameroonian and I started Julia just one year ago, I worked on a package named Notifyers.jl and I now I need some people to check the quality of my code.
Here is the link to my GitHub repository

2 Likes
2

Some remarks:

  1. add a folder test and a file runtests.jl with some unit tests
    You have some tests, commented in the main file of your Project, just move them to runtests.jl
  2. add a doc folder and some documentation
  3. ReadMe.md should be renamed to README.md
  4. The file License.txt should be renamed to LICENSE
  5. There should be no file index.md in the home folder of your project. It contains documentation, so move it to the doc folder
  6. Please indent your source code using four spaces and no tabs !!!
5 Likes
3

We now have the great GitHub - Eben60/PackageMaker.jl: GUI for PkgTemplates.jl: "Creating new Julia packages, the easy way" - made a bit simpler. take a look, it will make CI, Docs and everything ready for you

2 Likes
4

First the Quick start section must work, so replace Pkg.add("Notifyers") with Pkg.add(url="https://github.com/Gesee-y/Notifyers.jl.git")

Next you can try using JET.jl and Aqua.jl, which are great tools for catching basic mistakes (though they occasionally have false positives).

julia> using JET

julia> JET.report_package(Notifyers)

julia> using Aqua

julia> Aqua.test_all(Notifyers)
5

Thank you all for your feedback, I actually started working on every point you mentioned.
But, there no problem with the code ??

6

You’ll probably not get the kind of “code review” you’re looking for at this point quite yet. It would require someone having quite a lot of experience with your code (using it as a dependency, or contributing to it), before they could really tell you much about problems with the code design or the algorithms you used. For something like your OrderedDict.jl, I could tell you immediately that that’s a very “naive” implementation that’s not going to perform well and shouldn’t be used “in production” (and implementing ordered dictionaries is something I probably wouldn’t even attempt to do myself, but rather leave it to someone with an advanced CS degree – nothing wrong with making a simple implementation yourself, as a learning experience, though). For a package like Notifyers, things are not so obvious. Thus, you’re most likely to get comments on surface issues like tests on docs.

But those things really matter. Focus on making your code clean, logical, well-organized. Be a perfectionist when it comes to testing and documentation. You will find that focusing on form, your code will be “good” almost automatically.

For most code, algorithmic choices don’t matter that much. Make it work in the cleanest, most obvious and simple way first. When you run into performance issues, profile your code to find the bottlenecks, and look for algorithmic improvements in those places. Of course, if you have enough background and experience, you’ll be avoiding “obvious” algorithmically bad choices. But don’t worry too much for now.

If your ultimate goal is in game design, there’s the caveat that algorithmic efficiency does matter in that domain (as it does in other areas of “advanced” computing). But you’ll have a much better starting point if you focus on writing simple well-organized code. Then, when you identify bottlenecks, you can ask much more focused questions here on “what is the best way to approach this particular problem”. If you stick with this, you’ll ultimately learn the theory that you need. At some point, you’ll learn linear algebra and pick up an algorithms and data structures text book. Most people do that in college, so you might be a little early on that. I wouldn’t worry about it at this point. Don’t put too much pressure on yourself.

Again, I would focus on clean design, tests, and documentation. Spend time thinking about the logical structure of your code, and make notes about the design of your package (such notes are great to be worked into the Explanations section of your documentation, or into a CONTRIBUTING.md file where you explain development workflows and design choices to potential contributors.

I’m a big advocate of “documentation-driven” development, and to some extent also test-driven development (“TDD”). Write the documentation of new functions first. Maybe even offline, with pen and paper. Think about how it’s going to be used (what a “test” could be), what arguments it should have, how it’s going to interact with other parts of your system, whether your overall design still makes sense. Only then go to your computer to write the code.

In practice, I usually have an iterative process. Make some notes, write some example (test) code. Then write the actual code (docstring first). Complete the tests, fix bugs, fine-tune the documentation. The literal “TDD” approach, where you write a complete formal (failing) unit test and then write the code to make it pass always seemed a little ridiculous to me outside of very corporate environments. I would lean towards higher “integration tests” (an example program for how the whole system should work) as a start, and “unit tests” more to test for specific bugfixes. The main point it to think before you code.

I find that testing and documentation each take about twice as long as the initial implementation, but those 4/5ths of the work are what really matters in the long term.

Okay, with the philosophical recommendations done, some more surface-level tips:

  • Automate the formatting of your code. Use JuliaFormatter. Make sure that you can run it locally and run it before pushing any commits. Also run it in GitHub actions to make sure the code in your main branch is always properly formatted.

  • Set up documentation with Documenter as early as possible. You definitely want to have a complete API documentation immediately, but you’ll also want to start working on the free-form parts of the documentation (tutorials, design overviews) as early as possible. The Guide for setting up Documenter is a little bit outdated in some areas. The choice to start the guide with “building an empty document” is a bit unfortunate, in that it doesn’t really reflect how a real package should use Documenter. My recommendation is to look at existing Julia packages on Github, and copy what they do. (I guess I can recommend one of my own packages, like DocumenterInterLinks as a setup that I approve of ;-). For the hosting instructions, again there’s a bit of outdated information: Travis CI used to be the default provider for open source automated workflows, until that company was sold and stopped supporting open source. Now, the default provider is GitHub Actions. For most things where the guide talks about Travis, that also applies to GitHub actions (the section about SSH deploy keys in particular). In addition to the written material, I would also recommend the recent Documenter.jl deep dive on YouTube and maybe the Documentation Workshop from last year’s JuliaCon.

  • Keep the main Notifyers.jl clean. (Btw, you know that “notifyers” is a misspelling of “notifiers”, right? That might be deliberate, in that “notifiers” might be too generic as a package name, but you should have a “story” behind the name “notifyers” if you want to keep that). I would recommend that the file only contains const, export and include statements. And probably the precompile stuff (I’ve never use explicit precompilation in any of my package, and I’m not sure when/if it is really necessary). All structs, functions, etc. should be in files that are included in the main module. Look at how other Julia packages do this.

  • In your runtests.jl file, do not use include("../src/Notifyers.jl"). You want to make sure that you set up an independent environment for testing, either by putting an extras section in your main Project.toml file or (what I prefer) to put a different Project.toml in tests. You will then be able to test your package via Pkg.test. Although for a fast workflow in the REPL, I would recommend using TestEnv.jl. In any case, in your tests, you must have using Notifyers instead of directly includeing its code.

  • Split your tests into test_*.jl files inside the tests subfolder, and include them in some structured way in the runtests.jl file. There might be different ways to set that up. I’ve traditionally used SaveTestsets, but there are alternatives like TestItemRunner.jl. This aligns more with workflows in VS Code (if you were using VS Code, as probably most people are). I’m not that familiar with the details there, since I prefer development workflows centered around Neovim and Makefiles.

My general recommendation for all of this stuff is to look at existing Julia packages and emulate whatever they do. This also applies to code. It might be better to look at “smaller” packages for this. Large packages with many contributions and a long history have often organically grown complicated custom workflows and unusual design patterns that one wouldn’t use if starting from scratch. Seek out anything that’s small but looks polished.

14 Likes
7

Oh, ok
Guess I was too much searching for a technical review of my work.
I will first try to document it properly

1 Like
8

One more note on testing.

The test suite will not only help you on catching bugs and regressions. It is actually a collection of examples of code working, and thus a valuable supplement to documentation, especially for the developers of the package.

2 Likes
9

Thanks, so my tests should also serve as code example

1 Like