sudoku, added sample multiple dataset 1 and 2
sudoku, solved sublevel increment issue
sudoku, added multiple datasets
sudoku, replaced hardcoded symbolizeLevelMax variable with symbolizeLevel
echarruau | |
jjazeix | |
timotheegiet | |
AkshayCHD | |
amankumargupta |
sudoku, added sample multiple dataset 1 and 2
sudoku, solved sublevel increment issue
sudoku, added multiple datasets
sudoku, replaced hardcoded symbolizeLevelMax variable with symbolizeLevel
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
src/activities/sudoku/sudoku.js | ||
---|---|---|
521 | Previously hardcoded symbolizeLevelMax variable makes the first 7 levels to use symbols, replaced it with symbolizeLevel variable, and added setSymbolizeLevel function which checks sudoku will form with symbols or numbers at each level. |
Only a convention question here to decide at this beginning of coding.
src/activities/sudoku/sudoku.js | ||
---|---|---|
612 | Here within qtcreator it gives a warning telling that we should use !== which makes more sens because we do no need to convert type. |
src/activities/sudoku/sudoku.js | ||
---|---|---|
612 | yes, it would be better to have a strong comparison if QtCreator warns on it |
src/activities/sudoku/sudoku.js | ||
---|---|---|
570 | is there a preference for not setting it in the dataset directly instead of computing it when we initialize the level? |
src/activities/sudoku/sudoku.js | ||
---|---|---|
570 | It will cost us an extra variable in datasets for each level and then levels would not be simple arrays anymore but transform into maps and arrays and it's easy to compute it by code too, |
src/activities/sudoku/sudoku.js | ||
---|---|---|
570 | We need to think on how we will allow teachers/users to create datasets (and if it makes sense for the activity). Then, for this activity specifically, we probably don't need it... | |
573 | please add parenthesis here to avoid confusion |
Not sure if it was closed on purpose (or automatically because htere was a commit) but I don't see the latest required changes.
src/activities/sudoku/resource/1/Data.qml | ||
---|---|---|
24 | As discussed on the irc channel, we need to add a '.' at the end of the sentences |
src/activities/sudoku/resource/1/Data.qml | ||
---|---|---|
29 | is it still useful to separate imgName and extension? Just having imgName: "circle.svg"? |
src/activities/sudoku/resource/1/Data.qml | ||
---|---|---|
29 | Do you know why we did it initialy? I would prefer to have rectangle.svg. We have to think that some of these data will be filled up by teachers a day, the easiest the better ;) |
Working like a charm.
Once you will have made it tested by your "test group" as stated in the timeline it will certainly be ready.
Nice and clean job!
If you don't have more changes to do on this activity, this is good for me.
@AkshayCHD, @echarruau, @timotheegiet : do you plan on testing? If yes, I'll wait before merging else I'll merge it when I have time (next week)
Ran through all the datasets, looks good to go, just one thing can we add a third level in dataset 3, which contains both symbols and numbers. Currently all levels either contain symbols or numbers, which may lead to someone thinking that he can use only one of them at a time in a sublevel, which is not the case actually.
That it is technically possible to use both symbols and numbers in a level doesn't mean it is necessarily a good idea to do it by default.
While we can give the possibility in the future admin interface to mix both in custom-made datasets, I would prefer to keep them separated in datasets provided by default.
As Timothée just said I would also prefer to keep them separated in datasets provided by default.
Symbols are more aimed at children or beginners as number is more for advanced users.
The actual code is now perfectly generic and numbers are in fact symbols.