D17945: Tutorial screen added for odd even activity

Johnny Jazeix noreply at phabricator.kde.org
Wed Feb 27 12:20:53 GMT 2019


jjazeix added inline comments.

INLINE COMMENTS

> TutorialBase.qml:32
> +    property bool isEven: true
> +    property bool isFirstCorrect: true
> +

I still don't understand why you need 2 bools?
isEven (should be renamed to isEvenExpected) is set to true if the question is "choose the even number".
then, when you click on a button, if the number inside is even you should tell if it is good or not.

> TutorialBase.qml:69
> +                        message.text = qsTr("There is an error:" +
> +                                            " when divided by 2, it leaves remainder as 1." +
> +                                            "Therefore this is an odd number.")

missing space after the '.'?

> TutorialBase.qml:92
> +        isCorrectAnswer: !isFirstCorrect
> +                onPressed: {
> +                    if(!isCorrectAnswer) {

why is there 3 cases here and only one on the other button? Shouldn't they be "symetrical"?

> TutorialBase.qml:93
> +                onPressed: {
> +                    if(!isCorrectAnswer) {
> +                        message.text = qsTr("There is an error:" +

The checks order seems not logical.
There is a check on !isCorrectAnswer to display a text and two lines below, if it's not even, the text changes directly.
It would look more readable having:

  if(isCorrect) {
  }
  else {
    if(even) {
    }
    else {
    }
  }

REPOSITORY
  R2 GCompris

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

To: dekumar, #gcompris_improvements, jjazeix
Cc: echarruau, amankumargupta, jjazeix, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190227/8d2f4e5f/attachment.html>


More information about the kde-edu mailing list