The module, currently living in a sandbox (https://drupal.org/sandbox/gudm.d.haralds/1851918), needs a review.

This module provides currency calcuation designed for Icelandic users who wish to use currency-information from the Central Bank of Iceland. It retrieves currency-information from the Central Bank every time cron is run, storing information locally.

Although several module provide similar functionality, this one bases its functionality on the Icelandic Krona, and uses data from the Central Bank of Iceland. As far as I know, no other module does this. Note that the exchange of the Krona isn't available from the European Central Bank (http://www.ecb.europa.eu/stats/exchange/eurofxref/html/index.en.html).

To check out the module: https://drupal.org/project/1851918/git-instructions

CommentFileSizeAuthor
#3 currencycalc_iceland_screenshot.png20.74 KBgudm.d.haralds

Comments

PA robot’s picture

PA robot commented

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

tr’s picture

tr commented

I don't see why we need a separate module for every source of currency information. There's at least one other pending application that does the same thing but with a different bank: https://drupal.org/node/2186013

Rather than writing your own module with only a few features, why don't you write an add-on for the Currency module (https://drupal.org/project/currency)? Currency defines a plugin architecture so that you can write your own plugin to get currency information from your preferred source, in this case the Central Bank of Iceland. If you do this, you will get all the features of the Currency module for free, plus there will be a much larger community of users/developers to maintain and improve the code for you.

gudm.d.haralds’s picture

gudm.d.haralds commented
StatusFileSize
new currencycalc_iceland_screenshot.png20.74 KB

Maybe we don't need a separate module, that is right. However, things are more complicated than that.

My module provides a calculator in a block (see screenshot), which the currency-module does not. The 1.x-version does provide a calculator, but not nearly as powerful as the one my module provides. The 2.x-version does not provide such a calculator. If you take a look at the screenshot, you can see three input-fields. In each of those, you can enter a value, which will be converted to each of the other currencies. As far as I can tell, the currency-module does not provide a functionality like this.

In addition, the present module was written specifically to do the task of user-friendly conversion, while the emphasis of the currency-module is to provide an API.

However, it would be nice to have a module similar to currency_yahoo_finance, which would enable users of the currency-module to fetch exchange information from the Central Bank of Iceland. But the would be a separate project.

phoang’s picture

phoang commented
Status: Needs review » Needs work

I would recommend to write an add-on to Currency module or extend the use of Currency API as TR has mentioned.

Looks like you need to follow the branch naming convention such as: 7.x-1.x rather than 7.x-0.x instead. Here's the instruction about Release naming conventions.

Also please fix some errors on your module were found at http://pareview.sh/pareview/httpgitdrupalorgsandboxgudmdharalds1851918

FILE: /var/www/drupal-7-pareview/pareview_temp/currencycalc_iceland.info
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
5 | ERROR | It's only necessary to declare files[] if they declare a class or
| | interface.
6 | ERROR | It's only necessary to declare files[] if they declare a class or
| | interface.
--------------------------------------------------------------------------------
FILE: /var/www/drupal-7-pareview/pareview_temp/currencycalc_iceland.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
265 | WARNING | Only string literals should be passed to t() where possible
--------------------------------------------------------------------------------
gudm.d.haralds’s picture

gudm.d.haralds commented
Status: Needs work » Needs review

phoang: Thank you. I have now renamed the branch, and fixed those errors as well. - However, regarding writing an add-on, I ask you to address the arguments I presented above.

gudm.d.haralds’s picture

gudm.d.haralds commented

I did a little more research. The Currency API does not provide a way of fetching exchange rates for earlier dates, only the current date. As my module displays the change between to days, this functionality would be lost, should the Currency module be used.

GoRoost’s picture

GoRoost commented

( Not taking into context the discussion related to if this module is a duplicate )

- Coder (Code Sniffer) comes back clean, save one errer about a doc comment. However, verified that this doc comment does exist on the file it is naming.

- currencycalc_iceland-currencycalc_view.tpl.php : line 38 - t() your text

- currencycalc_iceland.module : Need to translate all of your text in errors, titles, etc

GoRoost’s picture

GoRoost commented
Status: Needs review » Needs work

(Changing status to "Needs work")

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.