<table><tr><td style="">AkshayCHD marked an inline comment as done.<br />AkshayCHD added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D21461">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21461#inline-121286">View Inline</a><span style="color: #4b4d51; font-weight: bold;">jjazeix</span> wrote in <span style="color: #4b4d51; font-weight: bold;">money.js:74</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I would use the dataset moneyItems, not directly the Constants one. Else the specific items won't be used.<br />
I guess you can use slice to copy the array (<a href="https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Objets_globaux/Array/slice" class="remarkup-link" target="_blank" rel="noreferrer">https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Objets_globaux/Array/slice</a>)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;"><a href="https://phabricator.kde.org/p/jjazeix/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;">@jjazeix</a> But there is a problem with using the dataset's moneyItems. If suppose we need to find change for 100 units paid, then the pocket values might not contain a 100 units note, it might instead contain one 50 units, two 20 units and one 10 units notes. So instead of just one 100 units note, tux will now hold 4 notes will would require alot of space and would hinder the visibility of individual notes.</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D21461#inline-121288">View Inline</a><span style="color: #4b4d51; font-weight: bold;">jjazeix</span> wrote in <span style="color: #4b4d51; font-weight: bold;">money.js:86</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I think the part: (avaliableCurrency[i].val > amountToBeCovered) is not needed because it should always be the case before breaking as we start from the higher value in a sorted array.</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">The dataset is sorted in ascending order that is why i added that condition, but yes it won't be needed if i sort it in descending order. Will do that</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R2 GCompris</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D21461">https://phabricator.kde.org/D21461</a></div></div><br /><div><strong>To: </strong>AkshayCHD, GCompris: Improvements<br /><strong>Cc: </strong>asagtani, jjazeix, kde-edu, GCompris: Improvements, sanjayshetty, parimalprasoon, harrymecwan, ganeshredcobra, nityanandkumar, echarruau, andreask, rahulyadav, narvaez, scagarwal, apol, timotheegiet, bcoudoin<br /></div>