Konstantin,

On 4/20/16 5:15 AM, Konstantin Kolinko wrote:
> 2016-04-20 8:04 GMT+03:00  <kfuj...@apache.org>:
>> Author: kfujino
>> Date: Wed Apr 20 05:04:19 2016
>> New Revision: 1740047
>>
>> URL: http://svn.apache.org/viewvc?rev=1740047&view=rev
>> Log:
>> Change the channel field to protected.
>>
>> Modified:
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatch15Interceptor.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/TcpPingInterceptor.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastService.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastServiceImpl.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReceiverBase.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReplicationTransmitter.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/bio/BioReceiver.java
>>     
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
>>
>> Modified: 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java?rev=1740047&r1=1740046&r2=1740047&view=diff
>> ==============================================================================
>> --- 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>>  (original)
>> +++ 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>>  Wed Apr 20 05:04:19 2016
>> @@ -144,11 +144,11 @@ public class ChannelCoordinator extends
>>              if ( Channel.SND_RX_SEQ==(svc & Channel.SND_RX_SEQ) ) {
>>                  clusterReceiver.setMessageListener(this);
>>                  if (clusterReceiver instanceof ReceiverBase) {
>> -                    
>> ((ReceiverBase)clusterReceiver).setChannel(getChannel());
>> +                    ((ReceiverBase)clusterReceiver).setChannel(channel);
>>                  }
>>                  clusterReceiver.start();
>>                  //synchronize, big time FIXME
>> -                Member localMember = getChannel().getLocalMember(false);
>> +                Member localMember = channel.getLocalMember(false);
>>                  if (localMember instanceof StaticMember) {
>>                      // static member
>>                      StaticMember staticMember = (StaticMember)localMember;
>> @@ -167,7 +167,7 @@ public class ChannelCoordinator extends
>>              }
>>              if ( Channel.SND_TX_SEQ==(svc & Channel.SND_TX_SEQ) ) {
>>                  if (clusterSender instanceof ReplicationTransmitter) {
>> -                    
>> ((ReplicationTransmitter)clusterSender).setChannel(getChannel());
>> +                    
>> ((ReplicationTransmitter)clusterSender).setChannel(channel);
>>                  }
>>                  valid = true;
>>                  clusterSender.start();
> 
> [....]
> 
> 
> What is the reason for this change ?  Are you tying to fix some bug here?
> 
> In general, I do not like this change.
> When the code uses getters it gives us more flexibility in the future,
> allowing to change the implementation.  In ReceiverBase class there
> are a lot of private fields. Why make 'channel' a protected one?

+1 to Konstantin's review. Making channel protected is okay (but might
be unnecessary), but the accessor methods should continue to be used.

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to