Is this bad OO programming? Passing this method

Question

Well, suppose I have a class, X and X are what have aggregate relationships with other objects. Lets claim to X - football stadium.

X is full of class spectators. However, the behavior of each viewer for a particular activity is different. Instead of IF statements, I want other behavior to be in the viewer class, so I can use dynamic binding.

However, the problem is that the behavior of the viewer affects the class of the “football stadium”. Therefore, I was thinking about going from the football stadium class, according to the method, to the Spectator class, so that the spectator class could do something in the class of the football stadium?

public class SoccerStadium{ SpecatorInterface s = new Spectator(); public void SpectatorBehaviour(){ s.doSomething(this); } public void doSomethingthingBySpecator(){ } } public class Spectator implements SpecatorInterface{ public void doSomething(SoccerStadium s){ s.doSomethingthingBySpecator(); } } 

I want to do this only to use dynamic binding and change the behavior in Specator.doSomething() so that I have many different types of SpectatorSuperClass as an attribute passed to SoccerStadium and then have different behavior.

EDIT: What if I passed the Stadium reference to the Specator through the Spectator constructor, instead of passing this ?

Answer 1

Your implementation is absolutely beautiful, I’ve seen such a thing before. Yes, you can stick to the stadium link by passing it through the Spectator constructor, which is likely to be cleaner than sending via the link every time you need it.

However, I do not really like it; I prefer inner classes. It is not entirely clear what you are trying to do, but something like this is possible:

 public class Outer { private int someVariable=0; public void someMethod(){ ExtendsInner ei = new ExtendsInner(); ei.innerMethod(); System.out.println(someVariable); } private void anotherMethod(){ someVariable++; } public abstract class Inner { public abstract void innerMethod(); } public class ExtendsInner extends Inner{ public void innerMethod(){ anotherMethod(); someVariable++; } } public static void main(String[] args){ Outer o = new Outer(); o.someMethod(); } } 

Unfortunately, then you will have to have all of your “viewer” classes inside your other class, which can lead to one very long file and therefore ugly code.

However, I think you should avoid doing both things, as this will certainly make your code overly complex.

Answer 2

This is not so much "bad programming" as crowded

Answer 3

I do not see a problem using this parameter. However, I don't like the call to new Spectator() , which was hardcoded in your SoccerStadium class. I suppose you should have a Factory with the createSpectator method, which could get a parameter indicating what type of viewer you intend to create.

Answer 4

For me, this bilateral bilateral relationship is bad news. What if the audience wants to go to the theater instead?

I would share the relationship by making the Stadium a subscriber to events posted by the viewer.

 public class SoccerStadium { ISpectator s = new Spectator(); public SoccerStadium() { s.DidSomething+=DoSomethingthingBySpecator; } public void SpectatorBehaviour() { s.DoSomething(); } public void DoSomethingthingBySpecator(object sender,EventArgs e) { Console.WriteLine("spectator did something"); } } public interface ISpectator { event EventHandler DidSomething; void DoSomething(); } public class Spectator:ISpectator { public event EventHandler DidSomething; public void DoSomething() { var ev=DidSomething; if(ev!=null) { ev(this,EventArgs.Empty); } } } 

... and therefore the Spectator now has a means of communication with everything that interests him, but he does not need to know anything about it.

Answer 5

As people said, there is absolutely nothing wrong with being closely connected and what you are doing. However, if you want to unleash a bit, use the classic visitor pattern.

 public interface SpectatorVisitor { ... void visit(Spectator spectator); } public class Spectator { ... public void accept(SpectatorVisitor visitor) { visitor.visit(this); } } public class Stadium { ... spectator.accept(new StadiumSpectatorVisitor()); } 

The signature of the visit method can be changed to accept a state object if you need to. Otherwise, you can simply define the appropriate methods in the Spectator class and force the visitor to collect the information necessary to change the stadium.

For instance:

 public class Spectator { private Team supports; public Team getSupports() { return supports; } public void accept(SpectatorVisitor visitor) { visitor.visit(this); } } public class SupportedTeamVisitor { private Map<Team, AtomicLong> supportCount = new HashMap<Team, AtomicLong>(); public void visit(Spectator spectator) { Team supports = spectator.getSupports(); if (! supportCount.contains(supports)) { supportCount.put(team, new AtomicLong(0)); } supports.get(team).incrementAndGet(); } public Map<Team, AtomicLong> getSupportCount() { return supportCount; } } public class Stadium { public long getSupportCount(Team team) { SupportTeamVisitor visitor = new SupportedTeamVisitor(); for (Spectator spectator : spectators) { spectator.accept(visitor); } AtomicLong count = visitor.getSupportCount().get(team); return (count == null) ? 0 : count.get(); } } 

Make sense?

Answer 6

As Matt said, what you describe is a visitor pattern. However, I don’t think your best alternative (as Falmarri said, such a design is usually closely related, and you end up adding a lot of logic to your business object, breaking SoC , SRP , etc.) . The fact that the behavior of each viewer for a particular activity is different does not mean that logic should be included (or passed through) the viewer's class. There are many different ways to avoid these IF messages. I suggest you go with something like this link , which is much more powerful than if instructions, a visitor template, or all other alternatives, and it is really easy to implement in another class and support all these OOP principles regarding goods (which exist for which for a reason).