williamgueiros / Brcpfcnpj

Validação,Formatação e Gerador Cpf/Cnpj em Elixir
MIT License
69 stars 17 forks source link

Improve random number generation #5

Closed gnbaron closed 8 years ago

gnbaron commented 8 years ago

Today, the way used to create random numbers in CNPJ or CPF generation can cause a "bug" when you use that in multiple tests or sessions, generating the same sequence of CNPJ's or CPF's for all of them.

i.e. Starts a iex session and generate a random CNPJ: iex(1)> Brcpfcnpj.cnpj_generate results in "21546853597400" If you starts another iex session and use the same function, you'll get the same result ("21546853597400").

To solve that problem, I suggest to change from random erlang module to rand module, which was released in Erlang 18 and improve these random number generation.

I did some tests using the rand module and this solves the problem, if you wan't I can do the changes.

williamgueiros commented 8 years ago

See that https://github.com/elixir-lang/elixir/issues/3212 Elixir 1.2 have a support the solution is the same you say, but if resolve using that solution the project doesn't work in elixir 1.0 , 1.1 and erlang 17

@diogobeda and @tiagoengel, do you like remove support elixir 1.0 and 1.1 ?

williamgueiros commented 8 years ago

To me :+1: and update version to 0.1.0 but using elixir method not erlang

tiagoengel commented 8 years ago

I don't see any problem on breaking the compatibility with erlang 17 and elixir < 1.2.

But if you don't want to break the compatibility, you can use the seed function, like so:

defmodule Randomise do 
  @on_load :reseed_generator 

  def reseed_generator do 
    :random.seed(:erlang.now()) 
  end
end

In fact, I think it's also a good practice to do that even using the :rand module (it also have a seed function).

gnbaron commented 8 years ago

For me it's ok still using the :random module to don't lose compatibility with elixir < 1.2, but doing that changes suggested by @tiagoengel.

diogobeda commented 8 years ago

Agreed with both of you. The least versions we lose compatibility the better.

diogobeda commented 8 years ago

Also, using reactions now just because I can. rs

williamgueiros commented 8 years ago

please see that in 12 mim http://youtu.be/YlNrWxH56_E OTP 19 putting the Random in deprecated mode .

I believe is good opportunity to resolve this problem and prepares to next version

diogobeda commented 8 years ago

Oh, thanks @williamgueiros. I didn't know about that. Having that in mind, I also think it's better to use rand.

williamgueiros commented 8 years ago

@tiagoengel @guilhermefloriani what do you thing, after see this video?

tiagoengel commented 8 years ago

If it will be deprecated I think would be better to use the :rand module

williamgueiros commented 8 years ago

ups remove support to elixir < 1.2, fail comment