H5P - SQL, Python, ....

Hi!

I'll gladly have a look when I find some time, but it was great if you checked for common issues such as violations of the H5P coding style guide that can easily be avoided.

Cheers,
Oliver

Hey!

I thought: Why not. Please note that this is not a thorough code review (so there may be more that I didn't spot right away as I didn't try to understand what the code was doing) and I have not looked at the running instance, at the code only.

Starting with H5P.CodeQuestion seemed like a good starting point, although I later noticed that this was just a non-runnable library.

• You might want to update the build chain slightly. I realize that it's from the h5p boilerplate repository (that was not updated a long time and) that has a couple of quirks.
• Commonly, in `semantics.json` files the indentation is using 2 spaces, not 4.
• You're releasing that as Joubel inside the LICENSE file? ;-)
• What's the .h5p file? An accident from building libraries?
• You should in fact check for coding violationse, e.g. mixing single quotes and double quotes, using `!=" instead of `!==`, missing semi-colons, etc. (or let eslint do so, cmp. a newer boilerplate for instance https://github.com/otacke/snordian-h5p-boilerplate)

entries/h5p-code-question.js

• H5P should be read only (line 5)

scripts/h5p-code-question.js

• Strings are hardcoded and not translatable.
• May be a matter of taste, but using member notation outside of the constructore `getDecode(code) { ... }` instead of `this.getDecodededCode = (code) => { ... };` inside the constructor feels nicer (e. g. line 20)
• Try to make functions fail early (line 21) as you do in line 39, this often helps to understand what a function does way easier, e.g. here: ` if (!code) { return ''; } or simply sanitize `code` like code = (code typeof 'string') ? code : '';
• Line 41 can be deleted if you change line 43 into `return this.runtime ?? this.runtime;`
• Line 42 is code commented out.
• Line 49: An empty function? How does calling createRuntime() work at all then? Is that defined again somewhere else?
• Line 72 is not necessary if you define `content` on line 75 instead - and using `let` here is not required as in many other places where `const` should be used.
• Line 76: Where does parent come from (guess I'd find out if looking at other code)? And is setting properties on it directly really a good idea? Either way, if UID is supposed to hint to a UUID, than that's good, because ids should be unique even if you have multiple instances.
• Line 82: Why bother to store that value in a variable if you only use it in line 83? And I guess there's a reason for the hardcoded alt tag? Similar in other lines
• Line 86: Don't be afraid of shorthands like `if (this.hasAssets)` in particular sincy you don't check for strict equality anyway
• Line 92: I like Toto, but I could not guess what that variable is meant to be used for :-D
• Function in Line 199 Blank lines for the win (and intelligibility).
• Line 260: What's `var` doing in here if you're using ES6?
• Line 311: If you want to return an object directly, you should wrap the return value in parentheses.
• Line 329: So, here the constructor function actually starts (would be right at the top if member functions were written in class notation).
• Line 347: Try to avoid using jQuery right away if possible - it will be removed from H5P at some point.
• Line 369: If you need UUIDs, you can simply call H5P.createUUID()

styles/h5p-code-question.css

• Line 2: What's `width: 100%` necessary? Am only looking at the code, not a live instance, and `width: 100%` is often not necessary.
• assigment [sic]

I then quickly glanced at some other repository and wondered: are those libraries copies of npm modules? If yes: Why bother to create H5P libraries out of them (that need their own maintenance, wrapping code, etc) if you could simply use them as dependencies?

Whoa! - Thanks!

I am very grateful for the feedback. As this is the first time I have programmed an H5P module (and to be honest, so much javascript), it helps me a lot.

As soon as I have some free time again, I will work through the feedback and update the modules.

Whoa! - Thanks!

I am very grateful for the feedback. As this is the first time I have programmed an H5P module (and to be honest, so much javascript), it helps me a lot.

As soon as I have some free time again, I will work through the feedback and update the modules.

> Every coder started small ...

:-) Glad to hear it :-D - Even though I've already done a lot of programming. But I've never been good friends with Javascript, because the approach to solve problems is often very different from what I'm used to in other languages.

For example, I wanted to declare the methods outside the constructor, but then ran into problems with `this` when I then passed these functions as callbacks in Promises, which I could only solve by using this strange way of declaring functions inside the constructor.

otacke

Tue, 04/11/2023 - 22:36

Permalink

I made a major rewrite of the plugins.I restructured some things (plugin dependencies and class structure), used the linter plugin to improve overall code quality.

The plugins can be found here https://codeberg.org/a_siebel/h5p-content-python-question and https://codeberg.org/a_siebel/h5p-content-sql-question . I produced a short introduction (language: german): https://youtu.be/Kb346xXr7f8

There is still work to do (especially translation which is still do be done) but I will now continue to test the plugin when creating Moodle courses.

I created new versions for the plugins and moved to github:

https://github.com/asbl/h5p-content-python-question
https://github.com/asbl/h5p-content-sql-question

These question types are actively used in my Moodle courses and are further developed based on student feedback.I completely revised the code again to reduce dependencies and make it as easy as possible to extend.

[画像:Python Question]

[画像:SQL Question Type]