Is coupling with strings "looser" than with class methods?

Question

I'm starting a school group project in Java, using Swing. It's a straightforward GUI on Database desktop app.

The professor gave us the code from last year's project so we could see how he does things. My initial impression is that the code is far more complicated than it ought to be, but I imagine programmers often think this when looking at code they didn't just write.

I'm hoping to find reasons why his system is good or bad. (I asked the professor and he said I'd see later on why it's better, which doesn't satisfy me.)

Basically, to avoid any coupling between his persistable objects, models (business logic), and views, everything is done with strings. The persistable objects that get stored in the database are hashtables of strings, and the models and views "subscribe" to each other, providing string keys for the "events" they subscribe to.

When an event is triggered, the view or model sends a string to all of its subscribers who decide what to do for that event. For instance, in one of the views action listener methods (I believe this just sets the bicycleMakeField on the persistable object):

    else if(evt.getSource() == bicycleMakeField)
    {
        myRegistry.updateSubscribers("BicycleMake", bicycleMakeField.getText());
    }

That call eventually gets to this method in the Vehicle model:

public void stateChangeRequest(String key, Object value) {

            ... bunch of else ifs ...

    else
    if (key.equals("BicycleMake") == true)
    {
                ... do stuff ...

The professor says that this way of doing stuff is more extensible and maintainable than having the view simply call a method on a business logic object. He says there is no coupling between the views and models because they don't know of each others' existence.

I think this is a worse kind of coupling, because the view and model have to use the same strings in order to work. If you remove a view or model, or make a typo in a string, you get no compile errors. It also makes the code a whole lot longer than I think it needs to be.

I want to discuss this with him, but he uses his industry experience to counter any argument that I, the inexperienced student, might make. Is there some advantage to his approach that I'm missing?

---

To clarify, I want to compare the above approach, with having the view be obviously coupled to the model. For instance, you could pass the vehicle model object to the view, and then to change the "make" of the vehicle, do this:

vehicle.make = bicycleMakeField.getText();

This would reduce 15 lines of code that are currently ONLY used to set the make of the vehicle in one place, to a single readable line of code. (And since this kind of operation is done hundreds of times throughout the app, I think it would be a big win for readability as well as safety.)

---

Update

My team leader and I restructured the framework the way we wanted to do it using static typing, informed the professor, and ended up giving him a demo. He's generous enough to let us use our framework as long as we don't ask him for help, and if we can keep the rest of our team up to speed -- which seems fair to us.

Answer 1

The approach your professor proposes is best described as stringly typed and is wrong on almost every level.

Coupling is best reduced by dependency inversion, which does so by a polymorphic dispatch, rather than hardwiring a number of cases.

Answer 2

Your professor is doing it wrong. He's trying to completely decouple the view and model by forcing communication to travel via string in both directions (view doesn't depend on model and model doesn't depend on view), which totally defeats the purpose of object-oriented design and MVC. It sounds like instead of writing classes and designing an OO-based architecture, he's writing disparate modules that simply respond to text-based messages.

To be somewhat fair to the professor, he is trying to create in Java what looks very similar to the highly common .NET interface called INotifyPropertyChanged, which notifies subscribers of change events by means of passing strings that specify which property has changed. In .NET at least, this interface is primarily used to communicate from a data model to view objects and GUI elements (binding).

If done right, taking this approach can have some benefits¹:

1. The View depends on the Model, but the Model does not need to depend on the View. This is appropriate Inversion of Control.
2. You have a single place in your View code that manages responding to change notifications from the model, and only one place to subscribe/unsubscribe, reducing the likelihood of a hanging reference memory leak.
3. There is way less coding overhead when you want to add or remove an event from the event publisher. In .NET, there is a built-in publish/subscribe mechanism called events but in Java you would have to create classes or objects and write subscribe/unsubscribe code for each event.
4. The biggest downfall with this approach is that the subscribing code can fall out of sync with the publishing code. However, this is also a benefit in some development environments. If a new event is developed in the model and the view is not yet updated to subscribe to it, then the view will still likely work. Likewise, if an event is removed, you have some dead code, but it could still compile and run.
5. In .NET at least, Reflection is used very effectively here to automatically call getters and setters depending on the string that was passed to the subscriber.

So, in short (and to answer your question), it can be done, but the approach your professor is taking is at fault for not allowing at least a one-way dependency between the View and Model. It's just not practical, overly academic, and not a good example of how to use the tools at hand.

---

^{¹ Search Google for INotifyPropertyChanged to find a million ways people find to try to avoid using magic string when implementing it.}

Answer 3

enums (or public static final Strings) should be used instead of magic Strings.

Your professor doesn't understand OO. For example having a concrete Vehicle class?
And to have this class understand the make of a bicycle?

Vehicle should be abstract and there should be an concrete subclass called Bike. I would play by his rules to get a good grade and then forget his teaching.

Answer 4

I think you have a good point, the magic strings are bad. Someone on my team had to debug a problem caused by magic strings a couple weeks ago (but it was in their own code that they wrote so I kept my mouth shut). And it happened... in industry!!

The advantage to his approach is that it might be faster to code, especially for small demo projects. Disadvantages are that it is prone to typos an the more often the string is used the more chance a typo code screw things up. And if you ever want to change a magic string, refactoring strings is harder than refactoring variables since refactoring tools might also touch strings that contain the magic string (as a substring) but are not themselves the magic string, and should not be touched. Also, you might need to keep a dictionary or index of magic strings so new developers won't start inventing new magic strings for the same purpose, and won't waste time looking up existing magic strings.

In this context, it looks like an Enum might be better than magic strings or even global string constants. Also, IDEs will often give you some sort of code-assist (auto-complete, refactor, find-all-references, etc...) when you use constant strings or Enums. An IDE won't offer much help if you use magic strings.

Answer 5

Using 'industry experience' is a poor argument. Your professor needs to come up with a better argument than that :-).

As others have stated, the use of magic Strings is certainly frowned upon, using enums is considered far safer. An enum has meaning and scope, magic strings do not. Aside from that, the manner in which your professor is decoupling concerns is considered to be an older and more fragile technique than used today.

I see that @backtodos has covered this whilst I was answering this, so I'll simply add my opinion that using Inversion of Control (of which Dependency Injection is a form, you'll run across the Spring framework in industry before to long...) is considered a more modern way to deal with this type of decoupling.

Answer 6

Let's be quite clear; there is inevitably some coupling in there. The fact that there is such a subscribable topic is a point of coupling, as is the nature of the rest of the message (such as “single string”). Given that, the question then becomes one of whether the coupling is greater when done via strings or types. The answer to that depends on whether you're concerned about coupling that is distributed over time or space: if the messages are going to be preserved in a file before being read in later, or are going to be sent to another process (especially if that's not written in the same language), using strings can make things much simpler. The downside is that there is no type checking; blunders won't be detected until runtime (and sometimes not even then).

A mitigation/compromise strategy for Java can be to use a typed interface, but where that typed interface is in a separate package. It should consist of just Java interfaces and basic value types (e.g., enumerations, exceptions). There should be no implementations of interfaces in that package. Then everyone implements against that and, if conveying the messages in a complex way is necessary, a particular implementation of a Java delegate can use magic strings as necessary. It also makes it much easier to migrate the code to a more professional environment such as JEE or OSGi, and greatly assists little things like testing…

Answer 7

What your professor is proposing is the use of "magic strings". While it does "loosely couple" classes between each other, allowing for easier changes, it is an anti-pattern; strings should very, VERY rarely be used to contain or control code instructions, and when it absolutely has to happen, the value of the strings you are using to control logic should be centrally and constantly defined so there is ONE authority on the expected value of any particular string.

The reason why is very simple; strings are not compiler-checked for syntax or consistency with other values (at best, they're checked for proper form regarding escape characters and other language-specific formatting within the string). You can put anything you want in a string. As such, you can get a hopelessly broken algorithm/program to compile, and it's not until it runs that an error occurs. Consider the following code (C#):

private Dictionary<string, Func<string>> methods;

private void InitDictionary() //called elsewhere
{
   methods = new Dictionary<string, Func<string>> 
      {{"Method1", ()=>doSomething()},
       {"MEthod2", ()=>doSomethingElse()}} //oops, fat-fingered it
}

public string RunMethod(string methodName)
{
   //very naive, but even a check for the key would generate a runtime error, 
   //not a compile-time one.
   return methods[methodName](); 
}

...

//this is what we thought we entered back in InitDictionary for this method...
var result = RunMethod("Method2"); //error; no such key

... all of this code compiles, first try, but the error is obvious with a second look. There are numerous examples of this kind of programming, and they are all subject to this kind of error, EVEN IF you define strings as constants (because constants in .NET are written to the manifest of each library in which they are used, which must then all be recompiled when the defined value is changed in order for the value to change "globally"). As the error is not caught at compile-time, it must be caught during run-time testing, and that's only guaranteed with 100% code coverage in unit tests with adequate code exercise, which is usually only found in the most absolute fail-safe, real-time systems.

Answer 8

Actually, those classes are still tightly coupled. Except now they're tightly coupled in a such a way that the compiler can't tell you when things are broken!

If someone changes "BicycleMake" to "BicyclesMake" then no one is going to find out that things are broken until runtime.

Runtime errors are way more costly to fix than compile time errors - this is just all kinds of evil.

Answer 9

Magic strings are bad - because the compiler can’t check for typos. If you look at MacOS you will find lots of places where a global variable is defined and you just know it is a string with the same characters. Like char* ThisNotification = “ThisNotification” - except it’s not official and the pox on you if you use the literal instead of the global variable. If you mistype the global variable name, the compiler tells you.

Now a common situation is that you have some data, it gets changed, and you expect that someone might want to react to the change. And you can’t know who all wants to know. And it will change. So there is a pattern that is often used: If your data is changed, you tell the world as precisely as you can that it is changed. The world listens to these notifications. The mechanism for passing notifications from listeners to observers should be generic. So a string interface is used to communicate the names of events. Please not string literals.

Imagine you have an address book. The user edits an address. And there is a list showing addresses in alphabetical order. Say 20 of your 10,000 adresses. So the displayer listens to changes and redraws the screen if one of the 20 adresses it displays is changed. The address sorter checks if the change affects the sort order. It moves the address in its sorted list and sends a “sort order changed” notification, saying “address x moved from position 3001 to 9620”. The displayer listens and if it was displaying either the address at location 3,001 or 9,620 it redraws its list. And so on. You would be in absolute trouble if your address book had to handle this itself.