Skip to content

Commit

Permalink
only compute path.to_s once
Browse files Browse the repository at this point in the history
  • Loading branch information
@tenderlove
tenderlove committed Mar 2, 2011
1 parent dc89e29 commit 1db4969
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion railties/lib/rails/engine.rb
View file
Expand Up @@ -382,7 +382,10 @@ def isolate_namespace(mod)

# Finds engine with given path
def find(path)
Rails::Engine::Railties.engines.find { |r| File.expand_path(r.root.to_s) == File.expand_path(path.to_s) }
path = path.to_s
Rails::Engine::Railties.engines.find { |r|
File.expand_path(r.root.to_s) == File.expand_path(path)
}
end
end

Expand Down

7 comments on commit 1db4969

@benmanns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this changes anything, as path is only used once in the method, anyway. Perhaps I'm missing something.

@jonahb
Copy link
Contributor

@jonahb jonahb commented on 1db4969 Mar 4, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are missing something :). Worst case, the block is executed for every engine.

@samuelkadolph
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.to_s was being called once or multiple times inside the block but outside the block it will only ever be called once.

@exviva
Copy link
Contributor

@exviva exviva commented on 1db4969 Mar 4, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't File.expand_path(path) be pre-calculated as well?

@jonahb
Copy link
Contributor

@jonahb jonahb commented on 1db4969 Mar 4, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that depends on the block parameter.

@vijaydev
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How so?

@jonahb
Copy link
Contributor

@jonahb jonahb commented on 1db4969 Mar 4, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, misread the comment!

Please sign in to comment.