Skip to content

Conversation

@sjoulbak
Copy link

@sjoulbak sjoulbak commented Jun 1, 2017

Hi @JakeStaTeresa I’m back with my fix :)

Until today we were using my fix branch of Currencies and I wanted to update Currencies to the newest version. Unfortunately the UndefinedFunctionError exception was still there.

So I started to investigate why this exception was thrown. In #12 ensure_compiled/1 was added to ensure that @currencies isn’t set when the dependency Poison isn’t compiled first.

To come back to the ensure_compiled/1 function, this function return an tuple with {:module, module} or {:error, :embedded | :badfile | :nofile | :on_load_failure}. I’ve tested what this if-statement does with these tuples,

iex> if {:module, Test}, do: true, else: false
true
iex> if {:error, :embedded}, do: true, else: false
true

As you see in this example, it returns true even when there’s an error.

A better way to catch this is by using the ensure_compiled?/1 function.

Ensures the given module is compiled and loaded.
Similar to ensure_compiled/1, but returns true if the module is already loaded or was successfully loaded and compiled. Returns false otherwise.

So I tried to use this function and let my application compile again hoping that this was fixed now. My conclusion of using ensure_compiled?/1 is that the application will be compiled! :) Unfortunately @currencies was nil, because Poison wasn’t compiled before Currencies. I also received an warning because the @currencies attribute wasn’t set at compile time.

warning: undefined module attribute @currencies, please remove access to @currencies or explicitly set it before access lib/currencies.ex:59: Currencies.all/0

When using Currencies in an application, you should also have Poison in your dependencies. Otherwise the attribute @currencies won’t be set at compile time. This makes Currencies dependent on Poison.

Hopefully I explained this time better why this dependency should have Poison as an dependency. Thanks for creating this dependency ❤️

Poison is needed for this dependency. `@currencies` is set at compile
time and therefore is Poison needed. This shouldn't be done after
compiling. Also removed the conditional since the conditional will
give runtime errors when there's no Poison dependency available.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 63087a9 on sjoulbak:feature/add-poison-dependency into 54cbaba on JakeStaTeresa:master.

@sjoulbak
Copy link
Author

@JakeStaTeresa Would you please review this PR?

@sjoulbak
Copy link
Author

@JakeStaTeresa Do you have any time to review this PR?

@JakeStaTeresa
Copy link
Owner

hi @sjoulbak! I saw a branch in your repo where you replaced ensure_compiled to ensure_compiled?. Did that change fix the issue for you? If yes can you send a PR from that branch?

@sjoulbak
Copy link
Author

sjoulbak commented Aug 16, 2018

@JakeStaTeresa As I mentioned in my first comment:

So I tried to use this function and let my application compile again hoping that this was fixed now. My conclusion of using ensure_compiled?/1 is that the application will be compiled! :) Unfortunately @currencies was nil, because Poison wasn’t compiled before Currencies. I also received an warning because the @currenciesattribute wasn’t set at compile time.

warning: undefined module attribute @currencies, please remove access to @currencies or explicitly set it before access lib/currencies.ex:59: Currencies.all/0

When using Currencies in an application, you should also have Poison in your dependencies. Otherwise the attribute @currencies won’t be set at compile time. This makes Currencies dependent on Poison.

@sjoulbak sjoulbak closed this Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants