Hello, Sorry for the question, but can you explain why you've constructed a loop with the `loop` statement for the token generation? If I'm not mistaken the `token` *value* of the hash parameter of the `exists?` method refers to `self.token` so basically the `token` attribute (i.e. column or method) of the current object being created (which is an in-memory object at that moment). I also assume that this value is `nil` since this column has no default value and `null` values are allowed. So the first time we presumably do not yet have any Users in the database with a `nil` token. Then the `break` instruction will not be executed, resulting to a second iteration for the loop, after a random token is being set for the in-memory object, right? So in the next iteration, we have a `token` value and we will `break` before setting another token if there are an existing user with that same token, am I still right? Then if not, we will continue like this again and again... And if so, we will end up with two users having the same token. Sorry for my bad English, but I hope my explanation makes sense... Can you explain what I misunderstood? Thank you. *(Thanks too for the videos!)*

In this example where users may already exist, I would have a rake task which would populate the tokens for the users. Once the tokens have been generated, I would have another migration which would set the column to a `null: false`. But I believe your confusion is valid as it should be `unless` instead of `if`. `break if self.class.exists?(token: token)` would cause an endless loop while `break unless self.class.exists?(token: token)` would give the desired functionality.

I understand the benefit of "code organization". but this is not slimming down the model, it's just moving stuff around. perhaps making it even harder to understand the User model because you now have to look at two different files. User isnt more SRP either, a central OO principle. I'm looking for a good reason why this is better in the sense of OO. Thanks! :)

Hello Steve, it really is the code organization which benefits from extracting the methods out. I've worked on similar applications in terms of size as the GitLab app and the fat models made it very difficult to work on. Grouping and moving ideas, like the friendships in the episode example, make the overall maintenance of the model easier over time. Yes, things are now in multiple files, but it ended up still being preferred over a large model. Ultimately, it does come down to the team's preference and their style of coding in which case I think should always be consistent on a per project basis. However, bringing a new person on board, it would be overwhelming to have to work in a large model like GitLab's user model. I consider most things, with exceptions, which makes something easier to maintain, lowering technical debt.

What OO gurus would suggest is that this object's "friendship" responsibility should be slit into POROs that can send messages to the original User object and be tested independently. Is there no other PORO way to do this?

So why not simply use an `until` loop, like so? ``` until self.class.exists?(token: token) self.token = SecureRandom.hex end ``` Thank you

I've started to read about the POODR book.. And indeed, it seems a better approach instead of simply modules... But since I'm new, I don't see yet how to figure out the abstraction/decomposition...

If by chance the token already exists then it will not generate the token. This probably would rarely happen on a small app with `SecureRandom.hex`, but on a larger table could be a concern. Another way to write it could be something like ``` def generate_token begin self.token = SecureRandom.hex end while self.class.exists?(token: token) end ```

Hi, Thank you for the quick reply... But isn't nil at the first time, so the loop is always entered at least once?

Haha yes, good point. Sorry, I'm winding down for the night and obviously not thinking correctly.

When I try this approach I get a **Circular dependency detected while autoloading constant** error when I run my app with foreman with resque. I do **not** get the error when I simply run `rails server`. In [this issue](https://github.com/resque/resque-scheduler/issues/646) it states: > You have to make sure all classes can be loaded without this circular autoloading. There is a bug in your application, it is not resque-schedule issue. >The classes can load fine in some ordering, but in others not. This is why you are getting this problem only occasionally. Sometimes the order your classes are loaded are different, and in those cases you get an exception. You need to make sure your classes can be loaded correctly in any order. Do you see how this problem could be created from using namespaced concerns? Thanks for the help.

It appears this is a common problem as seen [here](https://github.com/rails/rails/issues/14401), [here](https://github.com/rails/rails/issues/36054) and [here](https://stackoverflow.com/questions/37507321/rails-namespacing-concerns-based-on-model-name). The solution is to move your namespaced concern out of the `concerns/` directory and under the `models` directory as per [this answer](https://stackoverflow.com/a/50889849/1299792). This is the same directory structure [they are using at Basecamp](https://twitter.com/dhh/status/964864438938492930)