D18600: Improvements in ascending_order activity

[emmanuel charruau]

echarruau added a comment.


  You have been doing a great job, modularising the different ordering activity.
  I have added a few  comments to the code.
  For me your code is not generic enough, the variable names are still referring to numbers when they should refer to items or elements.
  Also there is still some parts where I can see some "if number...." we do not need them anymore.
  Last but important part, we should be able to present different sets of data for each level in ressources.
  Good job, this will be a very very useful activity for any class in the world, we are doing a lots of ordering from 6 years old up to 10 years old, and that in french, numeration or measures.

INLINE COMMENTS

> ordering_items.js:28
> +
> +function getRandomNumbers(numOfBoxes, upperBound, lowerBound) {
> +    while(num.length < numOfBoxes) {

Here we do not have numbers but items, we should have a more generic name

> ordering_items.js:29
> +function getRandomNumbers(numOfBoxes, upperBound, lowerBound) {
> +    while(num.length < numOfBoxes) {
> +        var randomNumber = Math.ceil(Math.random() * (upperBound - lowerBound) + lowerBound)

I can not see where the variable num is coming from, we need to avoid global variable for it to be readable and maintainable.

> dataset.json:4
> +    "letters": "a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z",
> +    "data": [
> +        {

I can not see what data is doing here.

> dataset.json:2
> +{
> +    "mode": "image",
> +    "images": [

Here like for all the other modes: alphabetical or number we should be able to add a set of data per level, so that we can increase the difficulty of the exercice.

> OrderingNumbers.qml:149
> +                            text: boxValue
> +                            visible: items.tileType == "number" || items.tileType == "alphabets"
> +                            anchors.horizontalCenter: parent.horizontalCenter

We should not have this anymore, your code should be totally generic

REPOSITORY
  R2 GCompris

REVISION DETAIL
  https://phabricator.kde.org/D18600

To: AkshayCHD, #gcompris_activities, jjazeix
Cc: jjazeix, echarruau, kde-edu, #gcompris_activities, sanjayshetty, parimalprasoon, harrymecwan, ganeshredcobra, nityanandkumar, andreask, rahulyadav, narvaez, scagarwal, apol, timotheegiet, bcoudoin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190418/8bd0abb1/attachment.html>