Skip to content

Commit

Permalink
Remove double alias chain AliasMethod(AliasMethod).
Browse files Browse the repository at this point in the history 
Remove final from methods putMethod and putAlias.
@enebo
enebo committed Feb 28, 2018
1 parent e2f575f commit 06570f1
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions core/src/main/java/org/jruby/RubyModule.java
View file
Original file line number Diff line number Diff line change
@@ -493,7 +493,7 @@ private void putMethod(String name, DynamicMethod method) {
* @param method
* @return method
*/ // NOTE: used by AnnotationBinder
public final DynamicMethod putMethod(final Ruby runtime, String name, DynamicMethod method) {
public DynamicMethod putMethod(final Ruby runtime, String name, DynamicMethod method) {
if (hasPrepends()) {
method = method.dup();
method.setImplementationClass(methodLocation);
@@ -1639,7 +1639,7 @@ public synchronized void defineAlias(String name, String oldName) {
* @param method
* @param oldName
*/ // NOTE: used by AnnotationBinder
public final void putAlias(String name, DynamicMethod method, String oldName) {
public void putAlias(String name, DynamicMethod method, String oldName) {
if (name.equals(oldName)) return;
putMethod(name, new AliasMethod(this, method, oldName));
}
@@ -1650,7 +1650,7 @@ public synchronized void defineAliases(List<String> aliases, String oldName) {
DynamicMethod method = searchForAliasMethod(getRuntime(), oldName);

for (String name: aliases) {
putAlias(name, new AliasMethod(this, method, oldName), oldName);
putAlias(name, method, oldName);
}

methodLocation.invalidateCoreClasses();

2 comments on commit 06570f1

@kares
Copy link
Member

@kares kares commented on 06570f1 Mar 1, 2018

Choose a reason for hiding this comment

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

just for reference (know you don't like finals) - these methods were 'marked' to intent for being internal.
thought of making them package visible and introducing an ugly __Internals class just for populator access.
but than hopefully not many outside consumers for the Java API so let's roll public 😉

@enebo
Copy link
Member Author

@enebo enebo commented on 06570f1 Mar 1, 2018

Choose a reason for hiding this comment

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

@kares yeah I think a comment is enough. Someone may use it but they will likely be using it for a reason and we may be wrong on not having made it public for real. OTOH if we decide to change it later and they complain they are on the hook to change it but we will also know that someone had the need (or thought they did). (note: just showing my rationale for why I dislike locking stuff down).

Please sign in to comment.