Dynamic Class Loading

 This article was originally posted on JRoller on June 30, 2005.

The other day, I wanted to write an Eclipse plugin (maybe more about that in a different post), in which I need to read a selected class file from the project I am working on and execute a method in it. Since I can not have my project in the classpath, I found out that the only solution is to have the class loaded dynamically. If there is a better solution in Eclipse, please somebody tell me.

Before starting to write my plugin, I decided to write a small test application, because I never used class loading before. So here is the class I want to load:

package hello;

public class HelloWorld
{
  public void run()
  {
    System.out.println ("Hello World!");
  }
}

To load it and execute the run method, you can then use the following lines of code:

        ClassLoader loader = new ClassLoader(getClass().getClassLoader())
        {
            public Class findClass(String name) {
                try
                {
                    String path = "C:\\mypath\\hello";
                    File file = new File(path, name + ".class");
                    RandomAccessFile raf = new RandomAccessFile(file, "r");
                    byte[] content = new byte[(int)file.length()];
                    raf.readFully(content);

                    return defineClass("hello." + name, content, 0, content.length);
                }
                catch (Exception e)
                {
                    e.printStackTrace();
                }
                
                return null;
            }
        };
        
        try
        {
            Class helloClass = loader.loadClass("HelloWorld");
            Object hello = helloClass.newInstance();
            Method m = helloClass.getMethod("run", new Class[0]);
            m.invoke(hello);
        }
        catch (Exception e)
        {
            e.printStackTrace();
        }

 I did not try the code on more recent java, but since the whole Class Loader API was in the process of being removed, I guess there are other ways to perform this nowadays. I tried asking ChatGPT to produce this code, and the result is quite similar, except it was using the URLClassLoader object which handles reading the file content for us.

Python: ruamel.yaml lib has a problem handling comments

 In our project, we are using the ruamel.yaml library for handling reading/writing YAML files. The reason we are not using yaml basic lib from Python is that ruamel handles better yaml standard, keeps the comments and formatting, and always dumps the keys in the same order.

However, since version 0.18.3, we had some strange behavior in our file dump. Some newlines were removed from some files. I opened ticket #492, with the following code that replicates the problem:

import ruamel.yaml

y = ruamel.yaml.YAML()

with open("organizational_units.yaml", "r") as file:

    ou = y.load(file)

with open("organizational_units.yaml", "r") as file:

    content = y.load(file)

content["organizational_units"] = ou["organizational_units"]

with open("test.yaml", "w") as file:

    y.dump(content, file)

with open("test.yaml", "w") as file:

    y.dump(content, file)

with open("test.yaml", "r") as file:

    y.load(file)

It is of course an oversimplified version of what we are doing in our project. We are normally loading several YAML files and combine them into one big model. Then, when we need to save changes into one file, we first reload it into memory in order to retrieve the original comments at the beginning of the file before replacing the old content with the new one.

Then you can see that we are saving our file twice. In fact, we are really performing a first save into an in-memory string stream, before logging the content in the file (at least in debug mode). Then we are saving it. Again, this code here is a simplification just to display the problem.

The problem occurs on the second save. The first works fine. Using this file as an example:

# Organizational Unit Specification

organizational_units:

- Name: root

  Accounts:

  - FirstAccount

  - SecondAccount

After the second save, we have this result:

# Organizational Unit Specification

organizational_units: -

  Name: root

  Accounts:

  - FirstAccount

  - SecondAccount

Noticed the missing newlines?

The last line of the code is loading the resulting file, just to show that we can not read it back.

After opening the ticket, I got the answer (on the same day, nice reactivity!) that it is in fact the duplicate of ticket #410. The #410 is a bit different, because it duplicates the complete structure, while we are only replacing a part of it. So maybe that is why our code was still working. I think the part that broke it is coming from this fix: "fix issue with spurious newline on first item after comment + nested block sequence".

As the developer explains, the issue is coming from the way the library is storing comments internally. It seems that comments are stored in different places, with the same reference. And when they are dumped, to avoid saving them several times, there is some internal bookkeeping going on. When we replaced reference to the top key, we broke some comments reference.

As a workaround, I restored comments reference around the top key:

comments = content["organizational_units"].ca.comment

content["organizational_units"] = ou["organizational_units"]

content["organizational_units"].ca.comment = comments

Worked for me...

JComboBox Editor Listening

This article was posted originally on JRoller June 3, 2005

To listen to edition event in the editor component of a JComboBox:

((JTextComponent)comboBox.getEditor().getEditorComponent()).getDocument().addDocumentListener(listener);

Moto: Alias Issue when Creating S3 Access Point is Fixed

 Two days ago, I discovered a small bug in the moto library that I use to unit test my lambdas on AWS. I needed to create an S3 Access Point with boto3, and while retrieving the its alias, I had different results when using the return value of create_access_point or get_access_point.

So I wrote a small unit test:

from moto import mock_s3control

import boto3

@mock_s3control

def test_access_point_alias():

    client = boto3.client("s3control")

    alias_from_create = client.create_access_point(

        AccountId="123456789012",

        Name="my-access-point",

        Bucket="MyBucket",

    )["Alias"]

    alias_from_get = client.get_access_point(

        AccountId="123456789012",

        Name="my-access-point",

    )["Alias"]

    assert alias_from_create == alias_from_get

I create an S3 Access Point, and retrieve its alias in two ways: from the response of the create_access_point function, and from the get_access_point function. On my moto 4.2.6, this test fails.

So I opened an issue on the project's repository. It was fixed and closed on the same day. That's reactivity!

AWS: The Next Token Pattern

When developing  for AWS, there is a pattern that you use each time a response to a service may return a lot of data. You get back some of the data, together with a token that you can provide to get another round of data.

As an example, let's take the service that returns the list of events from a Cloudformation template deployment. Here is how you would do it using Python and boto3:

cf_client = boto3.client("cloudformation")

response = cf_client.describe_stack_events(StackName="mystack")

# do something with the response

while response.get("NextToken"):

    response = cf_client.describe_stack_events(

        StackName="mystack",

        NextToken=response.get("NextToken")

    )

    # do again something with the response

However, there is one thing that I do not like with this pattern: code repeat. You call the service at two different parts of your code, with almost identical parameters. And you process the response in the same way, again in two places. If you have to fix something in this code, you have to remember to fix in both places.

The approach I use to have your code only once, is to take advantage of Python's capacity to pass parameters as a dictionary. Here is my approach to this pattern:

cf_client = boto3.client("cloudformation")

next_token = "FIRST TIME"

params = {"StackName": "mystack"}

while next_token:

    response = cf_client.describe_stack_events(**params)

    # do something with the response

    next_token = response.get("NextToken")

    params["NextToken"] = next_token

I store my parameter list in a dictionary, and I initialize the next token with something that is not empty. So the first time in the loop will always run. I can then call my service, without the token. After processing the response, I then read the next token and fill it in my parameter list. The second time round, it will call the service with my token.

Something funny happened when I started using this pattern into our production code. We tried using Bandit, which is a tool that analyze your code and looks for security issues. It would systematically flag my pattern with this error: 

[B105:hardcoded_password_string] Possible hardcoded password: 'FIRST TIME'

 Well, I have to slightly modify my pattern to avoid using the word token...

AWS: Simpler S3 File Deletes by Prefix

 I came across this code that deletes files in an S3 from a list of prefixes:

s3_client = boto3.client('s3')

for prefix in prefix_list:

    paginator = s3_client.get_paginator('list_objects_v2')

    file_list = paginator.paginate(

        Bucket=data_bucket,

        Prefix=prefix

    )

    for current_content in file_list:

        for current_file in current_content.get('Contents', []):

            current_key = current_file['Key']

            response = s3_client.delete_object(

                Bucket=data_bucket,

                Key=current_key

            )

The code creates an S3 client, and then, for each prefix in a list, it creates a paginator. Paginators are great because they help you avoid using all your memory when the list of files is big. Using this paginator, the code retrieves the list of all the files corresponding to the prefix, and deletes it.

Nothing bad in the code, it works nicely. My only remark here, is that there exists a simpler way. Instead of using the S3 client, you can create an S3 bucket resource. From there, you can simply delete all files listed under a prefix using a simple filter:

s3_resource = boto3.resource('s3')

bucket = s3_resource.Bucket(data_bucket)

for prefix in prefix_list:

    bucket.objects.filter(Prefix=prefix).delete()

Simpler!

AWS: Automatic Subscription Confirmation from SQS Queue to SNS Topic

 We have an architecture in AWS where different events from different accounts need to be sent to one central SQS queue. Since the events will cross both accounts and regions, one way to do it is to send them to a local SNS Topic. 

The SQS queue will have to subscribe to all those Topics, but we can not do it on the SQS side, since it does not know each time someone pops out a new account. However, the problem with having the SNS Topics create the subscriptions, is that they are waiting for confirmation from the SQS queue.

Since we already have a lambda waiting on the other side of the queue, handling all the events, we added a small code to handle the subscription confirmation as well. Here it is:

import json

import urllib.request

def lambda_handler(event, context):

    for record in event["Records"]:

        body = json.loads(record["body"])

        if body.get("Type") == "SubscriptionConfirmation":

            handle_subscription_confirmation(body)

def handle_subscription_confirmation(message):

    url = message["SubscribeURL"]

    with urllib.request.urlopen(url) as response:

        print(response.read())

I find it strange that the Cloudformation template that we use to create the subscription does not handle the confirmation as well. Or maybe not cross-account?

Other Comment Stories

 The following articles were originally posted on JRoller, and have as a common theme the code comments.

This first article was posted on August 24, 2005, with the title "Successful comment"

Yesterday, I was looking at some method, trying to understand what it was doing. I decided to look at the comments above the method signature, which might hopefully be of help. What I found was the following line:

// Returns true if successful

Well, that was the only information that I could have found without this help by just looking quickly through the code. Then I asked myself: does it mean that it returns false if the method fails? It reminds me of this story about two mathematicians trying to find out the color of a cow by looking at it grazing the grass from one of its side. If the side they see is white, does it mean that the whole cow is white? Then they agree that the cow is white on its left side.

This second article was posted on March 29, 2006, with the title "When is refactoring needed?"

When you see comments like this one, you know that a refactoring is not too far away:

  //Does anything, except storing
  registry->StoreNetwork();

This last article was posted on October 18, 2006, with the title "Find the root cause"

After eight years in Hungary, I am back to France. I have lots of new code to learn, so hopefully lots of new material for this blog.

Now something about bugfixing. You probably know that if you have to fix a bug, you should look for the root cause, and fix the bug at the root. That's the theory, but there are cases when you just can't do that, and the comment I found in this code is a good example:

if (cd == null) // It can happen. I submit 30 mn before the deadline!

Well, it's OK if you go back later and make a real fix.

Incomplete Class in Java 5.0 Released Version?

 This article was posted originally on JRoller July 22, 2005

I just found this comment in the WindowsFileChooserUI class, in the Java 5.0 source code:

    // The following are private because the implementation of the
    // Windows FileChooser L&F is not complete yet.

Did they forget to remove those lines? Or is the class really not complete? Did they lack time before the release? Did they have time to test it at all? Would we expect to have a complete version by Mustang? or Dolphin?

Following this comment is a list of private constants about Windows version. I would have expected to have them in a package private class, probably in the form of an enum. So I guess the class is really not complete, and this constants might be duplicated in some other places. Or is the FileChooser the only place in the whole Windows L&F where they have to make a difference between Windows versions?

Funny thing, I checked the implementation of the com.sun.java.swing.plaf.windows.WindowsFileChooserUI class in my Java 17, and the comment is still there. However, the constants disappeared. So on one side, there are no more differences in the L&F between windows versions. On the other side, nobody dared to remove the comment.

Comment Stories

 This article was posted originally on JRoller on April 27, 2005.

Comments are meant to be read. Sometime, they tell a story. The story of the code that follows them. Using Version Control tool, you can even retrieve the main characters.

Comment Story 1: John and Eduard

During a design meeting, John explains to Eduard how to modify the code he wrote, to include a new feature. They agree that some switch statement would be needed at some point in the class. But Eduard does not like switch statements. Eduard prefers the long list of if/else statements. He is a fervent Conditional Spaghetti adept. So he writes the code his own way, and include the following comment at the top of its code:

// sorry John, I hate the case structure...

Comment Story 2: Robi and Andras

Robi likes complex code. He likes long methods and overusing design patterns. His pride is a 400 lines long method, introduced by the following comment:

//the fair dinkum! select appropriate editor, handle selections, wash, iron, f*ck etc.

Robi leaves the project, and his code is handed over to Andras for maintenance. Andras has to fix bugs in Robi's code, and he doesn't like it. He has several sleepless nights, dreaming about monster methods managing everything in his house, ranging from dish washing to bringing down the garbage. For fear of breaking anything, Andras will try to change as less code as possible in Robi's piece of art. On one inspired day, he will add the following comment at the beginning of a 2500 lines of code long class:

/**
 * @author  K. Robi
 * ^^^^^^^^^^^^^^^^ you can tell ;-)
 */

Comment Story 3: Robi is angry
Robi works with JTables. He is trying to make something not really easy: the table should sort if you click on a column, and an arrow should show on the table header on which column and in which direction it is sorting. Being a big Design Pattern fan, he is putting quite some of them, and of course over-complicating the whole design. But this is not the point here. The point is that for some reason, something does not work when the reordering of columns is not allowed. Was it a Java bug? Or is it a misunderstood feature of JTable? I don't know, but what I know is that Robi spent some time on it. You can tell because he left several lines of code commented out:

//                    getTableHeader().invalidate();
//                    validateTree();
//                    invalidate();
//                    revalidate();
//                    repaint();

And then he got angry. REALLY angry. So angry that he felt he has to put the following comment in his code:

    //some fucking ugly workaround for the case when fucking column reordering is not allowed.
    //damn fucking strange, but none of these fucks above work and i'm fucking tired of fucking around
    //with these fucked-up tables! JTable fucks!
    //fuck that!
                

All this followed by those mysterious lines of code that I would be too scared to remove:

                    if(!getTableHeader().getReorderingAllowed())
                    {
                        TableColumn col1 = getColumnModel().getColumn(0);
                        int w = col1.getWidth();
                        col1.setWidth(w-1);
                        col1.setWidth(w+1);
                    }

Comment Story 4: Gabor and Peti
Gabor is new to the project. At some point during his learning curve, he has to fix quite a complex bit of code, a method of over 100 lines checking that some value fulfills all requirements before being submitted to the data store. It's quite a lot of tests, but still, he is not sure. Was everything tested? So in case someone comes up with another idea, he puts in the following comment in the code, at the end of the method:

    // ??? anything else?

Peti did read this comment, and found it funny. He actually felt the need to answer the question, and to show some positive and optimistic reaction to comfort Gabor that there would probably not be, but with the firm knowledge of experience that says "who knows...". He inserted the following comment:

    // Oh, no.

Comment Story 5: Baby come back
Sometimes, developers get bored, because the code they have to write does not present any challenge or fun. So they find way to have fun, and one way to do it is to put original comments in the code. That's probably what happened to Peti, who wrote some code which copies some value, use the original storage for some calculation, then restore the original value to its storage. In normal time, he would have put a comment like "Putting the original value back". But instead, he wrote the following:

  // Oh, baby come back

Comment Story 6: Peti talking to himself
It happens that comments are a mean of discussion between developpers, as it happened in Comment Story 4 between Gabor and Peti. I found such a comment spawned on three lines which seemed to be a dialog. But looking back into the version tree, it turned out that these three lines were written by the same person:

        // maybe only atm?
        // what do you think?
        // i'm not sure.

Probably still waiting for an answer...

Pascal Lover

 This article was originally posted on JRoller on June 11, 2004

I have a friend/colleague whose favorite language is Pascal. However, like half of my company, he had to learn Java, and of course had to use it as well. In his firt pieces of code, he tried to use some of the tricks he gathered from his Pascal experience. I think that most of us are doing the same. My first C program looked like Pascal, and my first Java program looked more like a direct translation from C++. It takes time to understand the philosophy of a language and use it the way it was meant to be used.

One of the trick he was using comes from the fact that Pascal is not efficient in String comparison. So my friend tried to avoid the following code:

if (myString.equals("value1"))     {       //value1     }     else if (myString.equals("value2"))     {       //value2     }

Instead, he rather used his own method, which was using a search of a substring within a String. Here is how it looks like:

switch (Util.whichOne(myString,"@value1@value2@"))      {      case 0:         //value1       break;      case 1:         //value2       break;      }

Seeing this code during my code review, I ran a small speed test, and of course the Pascal way was slower (by a factor of 7) than the simple condition list. So not only the philosophy is different between languages, but the possible optimisations too.

Not only between languages, but also between versions of the same language. In the past, the String's equal() method looked something like that:

public boolean equals(Object o)   {     if (!(o instanceof String)) return false;     return (compareTo((String)o) == 0);   }

Many programmers, including myself, were use to directly call the compareTo() method instead. However, in more recent versions of Java, the equals() method is an entity of its own, optimised for speed, and is faster on average than the compareTo() if what you are interested in is really the boolean return value. One of the optimisations is to check if the two Strings are of different size. In that case, equals() can directly return false while compareTo() still needs to calculate the difference between the two first non-equal characters.

All that comes back to the usual advice: if you really need to optimise something, check that what you are doing is really an optimisation.

The advice on using equals() for String is even more true today, when Java uses an inline C function to perform the operation.

Conditional Spaghetti

 This article was originally posted on JRoller on June 24, 2004

Often, having a lot of if/else structures one after the other in a code means that something is wrong with the design. It's what Kent Beck would call a Bad Smell. And usually, you can remove the conditions by using polymorphism. However, it is not always possible, or using inheritance would complicate the design too much.

Here, I want to speak about a certain kind of conditional spaghetti, where the list of if/else is quite long and the body is the same for each branch. This case can be viewed as a conversion between 2 values.

Here is a code that I've seen in one of the programs I had to review. It decides what icon to use for a given object type. There were something like 80 conditional branches.

if (getType().compareTo("PSTNGateway") == 0)     node.setIcon(new ImageIcon(       RCMGR.getImage("Image.Gateway")));   else if (getType().compareTo("PSTNNode") == 0)     node.setIcon(new ImageIcon(       RCMGR.getImage("Image. Node")));   else ...

My solution for this kind of code is to use a static HashMap for converting the object's name to its icon. Here is the HashMap declaration:

private static HashMap nodeTypes = new HashMap(50);   static {     nodeTypes.put("PSTNGateway", "Image.Gateway");     nodeTypes.put("PSTNNode", "Image.Node");     ...   }

Then, the 160 lines of conditional code becomes a simple two lines of code:

node.setIcon(new ImageIcon(RCMGR.getImage((String)     nodeTypes.get(getType()))));

Not only it is easier to read and maintain, but the 40 String comparisons on average become a hash code calculation and a couple of comparisons. The same technique can be applied to a Factory. From the same program, the code that was creating the nodes according to some parameter String looked like this:

if (cPar.getType().compareTo("BSC") == 0)     newNode = new TDBSC(cPar.getName());     else if (cPar.getType().compareTo("RNC") == 0)     newNode = new TDRNC(cPar.getName());     else ...

Now using the HashMap to convert between the node's name and the corresponding class:

private static HashMap nodeTypes = new HashMap(150);   static {     nodeTypes.put("BSC", TDBSC.class);     nodeTypes.put("RNC", TDRNC.class); ...   }

Class nodeClass = (Class)nodeTypes.get(cPar.getType());   Constructor init = nodeClass.getConstructor(new Class[] {String.class});   newNode = (TDTreeNode)init.newInstance(new Object[] {cPar.getName()} );

As a final note, I want to mention that the HashMap can be totally avoided by using naming convention. Then, the node class for example can be inferred with a simple formula:

Class nodeClass = Class.forName(PACKAGE_NAME + CLASS_PREFIX + nodeType);

This article was written in Java 1.4. Now, of course, using generics, you can avoid all those ugly castings. I used this pattern several times following this article. The gain in number of code lines is big, but it comes also with a notable gain in performance.

Horror Code: Boolean Parsing Fest

 This article was originally posted on JRoller on September 21, 2015

A small gem I found the other day:

String value = ...
boolean booleanValue = Boolean.valueOf(value.toLowerCase()).booleanValue();

A lot of useless stuff:

• toLowerCase is useless because the Boolean class is using equalsIgnoreCase.
• booleanValue is unnecessary because of automatic unboxing.
• Boolean.valueOf creates a useless temporary Boolean object.

Why does it seem so hard to parse a boolean?

boolean booleanValue = Boolean.parseBoolean(value);

And this works even if value is null.

Horror Code: FIFO Stack and LIFO Queue

This article was originally posted on JRoller on February 4, 2014 

Recently, I found this class in one of our applications:

 public class FIFOStack extends Vector {

	public synchronized Object push(Object o) {
		add(o);
		return o;
	} 

	public synchronized Object pop() {
		if (isEmpty()) {
			return null;
		} 
		return remove(0);
	} 

	public synchronized boolean empty() {
		return isEmpty();
	} 
} 

I was imagining the developer thinking: "If only I had a class like Stack, except it would work in FIFO mode". The code is really old, so there was no Queue implementation in the JDK at this time. But still, definitions of Stacks and Queues are way older than the JDK.

Out of curiosity, I looked in the code for LIFO Queues. To my surprise, I found 2, both in the JDK itself. One caught my attention, due to the comment preceding it. See for yourself, in the SynchronousQueue class:

    
    /*
     * To cope with serialization strategy in the 1.5 version of
     * SynchronousQueue, we declare some unused classes and fields
     * that exist solely to enable serializability across versions.
     * These fields are never used, so are initialized only if this
     * object is ever serialized or deserialized.
     */

      

I do not have a Java 1.5 around to check, but if I understand correctly the comment, serialization of SynchronousQueue in Java 1.5 was too tied to its implementation. So when they changed it in Java 1.6, they had to create the old LifoWaitQueue internal class just so that serialization would work with previous Java versions. Backward compatiblity can make to ugly code sometimes. The funny part is that the internal LifoWaitQueue and FifoWaitQueue classes were replaced by TransferQueue and TransferStack. Even the best of us can make naming mistakes...

Of course, the important thing to understand this post is that Stacks are supposed to be LIFO, and Queues FIFO.
I checked on my Java 17 for the SynchronousQueue class, and this comment, together with its LifoWaitQueue, are still there in the code. Time for deprecation?

Horror Code: Get Simple Class Name

 This article was posted originally on JRoller on January 29, 2014

You probably heard of one method of Class that give the simple name of the class, without all the packages. Even if you hadn't, you probably know a simple way to get the sub-String following the last dot. Well, I know someone who doesn't:

 	public static String classnameNormalize(String classname) {
		String s = new String(classname);
		int n;
		while (true) {
			n = s.indexOf(".");
			s = s.substring(n + 1, s.length());
			if (n == -1) {
				return s;
			} 
		} 
	} 

The function that gives the name of the class without its packages is Class.getSimpleName(). But if you don't know about it, and you need to get the substring after the last dot, it is better to use the String.lastIndexOf() method instead of this whole loop.

Double Timeout

 This article was posted on JRoller on October 9, 2013

The other day, I needed to check if a configuration parameter giving a timeout in number of seconds was really used by our communication layer. I came to the point in the code where we are sending a request and giving the timeout parameter to the lower layer method (a standard market product). The thing that caught my attention was that the method timeout parameter was of type double.

So I decided to decompile the code just to check why on earth they decided upon a double. This led me to the following conversion method (this is decompiled code, so the constants are already inlined):

static final long toMillis(double d)
{
    if(d == 0.0D)
        return 0L;
        
    double d1 = (d + 0.00050000000000000001D) * 1000D;

    if(d1 >= 9.2233720368547758E+018D)
        return 9223372036854775807L;

    return (long)d1;
}

So yes, it is converted to a long in some strange way. Maybe they want to cope with milliseconds or even nanoseconds in a future version?

Horror Code: You Said Naming Conventions?

 This article was originally posted on September 19, 2013

I remained speechless...

public final class AbstractButtonList

The class name says abstract, but the modifiers say final. Quite misleading...

Horror Code: How to Align two Labels

 This article was originally posted on JRoller on September 13, 2013

Or rather how not to. This is a code a colleague found on our GUI. The developper probably did not know how to align two components, so he decided to do it his own way. There is no easy way to set the size of a JLabel, but there is a constructor for JTextfield on which you can set a size in number of characters. So he took a JTextfield, removed the border, set it as non editable, and it becomes magically a JLabel to the outside world.

        final JPanel lbgpanel = new JPanel();
        lbgpanel.setLayout(new FlowLayout(FlowLayout.LEFT));
        final JTextField lbgLabel = new JTextField("Background :", 7);
        lbgLabel.setBorder(null);
        lbgLabel.setEditable(false);
        lbgpanel.add(lbgLabel, BorderLayout.EAST);
        lbgpanel.add(_colorBackgroundBt, BorderLayout.EAST);
        lbgpanel.add(_resetBg, BorderLayout.EAST);

        final JPanel lfgpanel = new JPanel();
        lfgpanel.setLayout(new FlowLayout(FlowLayout.LEFT));
        final JTextField lfgLabel = new JTextField("Foreground :", 7);
        lfgLabel.setBorder(null);
        lfgLabel.setEditable(false);
        lfgpanel.add(lfgLabel, BorderLayout.EAST);
        lfgpanel.add(_colorForeGroundBt, BorderLayout.EAST);
        lfgpanel.add(_resetFg, BorderLayout.EAST);

As a side note, I never understood the use of the BoderLayout.EAST constant in a FlowLayout.

Reading these old articles, I can see now that I was never really explicit in my explanations... Here, the real WTF is not that he used a JTextfield instead of a JLabel, although I'm sure the JLabel would have been a better fit. It is rather the use of a BorderLayout constant with a FlowLayout, that luckily would just ignore it. It might be because of a previous try with a BorderLayout, but then, setting all components on the eastern border will not yield the expected behavior...

Horror Code: ToString Overflow

 This article was originally posted on JRoller on June 12, 2013

A colleague of mine showed me this code from one of the libraries we are using:

    @Override
    public String toString()
    {
        final boolean reuse = false;
        try
        {
            this.ssc.socket().getReuseAddress();
        }
        catch (SocketException exception)
        {
            LOGGER.error(this+ \" SocketException\", exception);
        }

        return String.format(\"AccId[%1$10s] Port[%2$s] reuse=%3$s backlog=%4$s\", hashCode(), this.listenPort, reuse, BACKLOG); 
    } 

The strange part is the code trying to perform some socket operation during a toString. My guess is that they wanted to return something into that poor boolean that just seem stick to a false value. But what does really happen if you get that dreaded SocketException? Do you see it? Yes, that’s a Stack Overflow!

For those who did not see it at the time, here is the reason 10 years later: the logging of (this + something) will cause a call to the same toString method.

Horror Code: Empty Integer

 This article was originally published on JRoller on July 20, 2012

I just found this interesting piece of code:

  Integer oldGroupId = ...
  Integer groupId = (oldGroupId.toString().trim().equals(GUIConstants.EMPTY_STRING)) ? NumberConstants.ZERO_INTEGER : oldGroupId;

At first, I was not quite sure why he used the Integer class. Either he would expect the value to be possibly null, and a nice NPE would result of the following code. Or he wanted to use the toString method, ignoring that a static version exists in the Integer class.

And then, he trims the String, maybe not trusting that those guys developing Java would return a clean String. Better be on the safe side.

But the nicest touch of all, is that all of this is for testing if the trimmed String is empty. One must now take some minutes to consider what possible value of an integer can give back an empty String. When one finished pondering, one will just remove all this jibber-jabber and replace it with a simpler int assignment.

Horror Code: Interning String Literals

 This article was originally posted on JRoller on June 29, 2012

I found this kind of code in our application:

  public static final String CONSTANT_STRING = "Some value".intern();

Interning a String means storing it into a common pool of Strings managed by the JVM, so that if I already used this String somewhere, I will only get back its reference. It may be a good idea sometimes, but you have to know that all String literals are already interned by the JVM. As useless as calling toString() on a String.

Horror code: Double salto

 This article was posted originally on JRoller on February 24, 2012

A colleague noticed this code yesterday, which is the Java equivalent of jumping a double salto, then falling back at the same place from where you started:

  private List allHosts;

  public void myMethod(Host theHost) {
    Host myHost = allHosts.get(allHosts.indexOf(theHost));
    ...
  }

I did not put much context in my original post. The Host object was not comparable, so the indexOf operation will look for the object by reference. It would then retrieve the same Host object passed as a parameter. Since Host objects were always to be found in the list, and Exceptions were not even handled, we removed the line, and the program just worked as before.

Horror code: Eight-State Boolean

This article was originally posted on JRoller on May 27, 2011

This is probably the worst code I ever seen.

Imagine an application with a table, where the user can filter lines according to some criterium. Someday, someone asked to be able to combine criteriums with "OR" or "AND" operands. The developer at the time thought that since the operand can only have two values, it is logical to store it as a boolean.

Some later day, the user probably complained that he can not force priority of the operands, say with parenthesis. Or maybe it was the developer's idea. So he added the possibility in the user interface to choose between eight types of operands, in a combo box: OR, AND, OR(, AND(, )OR, )AND, )OR(, )AND(. And here are the constants declaration:

    public final static Boolean  OR  = Boolean.TRUE;
    public final static Boolean  AND  = Boolean.FALSE;
    public final static Boolean  _OR  = Boolean.FALSE;
    public final static Boolean  _AND  = Boolean.FALSE;
    public final static Boolean  OR_  = Boolean.TRUE;
    public final static Boolean  AND_  = Boolean.FALSE;
    public final static Boolean  _OR_  = Boolean.TRUE;
    public final static Boolean  _AND_  = Boolean.FALSE;

Notice how all those values still fit nicely into a Boolean. Now we need the code that parses the operand String from the combo:

    static public Boolean getOperande(String aOp)
    {
        if (aOp == null)
            return OR;
        if (aOp.equals("OR"))
            return OR;
        else
            if (aOp.equals("AND"))
                return AND;
            else
                if (aOp.equals("AND("))
                    return AND_;
                else
                    if (aOp.equals(")AND"))
                        return _AND;
                    else
                        if (aOp.equals("OR("))
                            return OR_;
                        else
                            if (aOp.equals(")OR"))
                                return _OR;
                            else
                                if (aOp.equals(")AND("))
                                    return _AND_;
                                else
                                    if (aOp.equals(")OR("))
                                        return _OR_;
                                    else
                                        return _OR;
    }

And the opposite operation for displaying nicely the filter:

    static public String getOperande(Boolean aOp)
    {
        if (aOp == OR)
            return "OR";
        else
            if (aOp == AND)
                return "AND";
            else
                if (aOp == AND_)
                    return "AND(";
                else
                    if (aOp == _AND)
                        return ")AND";
                    else
                        if (aOp == OR_)
                            return "OR(";
                        else
                            if (aOp == _OR)
                                return ")OR";
                            else
                                if (aOp == _AND_)
                                    return ")AND(";
                                else
                                    if (aOp == _OR_)
                                        return ")OR(";
                                    else
                                        return "OR";
    }

Both methods apply the same principle for using at best the horizontal size of your screen. I wonder if the JVM will actually optimize out the useless if clauses...

For completeness, here is part of the code that will apply the operands:

    protected boolean newApplyFilter(int i, int aRow)
    {
        boolean previousRes = true;
        boolean res = true;
        Boolean op = OR_;
        for (int j = i ; j < critereList.size() ; j++)
        {
          Critere c = (Critere)critereList.get(j);

          res = ...;

            if (j != i)
          {
              if (op == AND)
                  res = previousRes && res;
              else
                    if (op == OR)
                      res = previousRes || res;
                  else
                        if (op == AND_)
                          return (previousRes && newApplyFilter(j, aRow));
                      else
                            if (op == OR_)
                              return (previousRes || newApplyFilter(j, aRow));
                          else
                                if (op == _AND)
                                  return (previousRes && newApplyFilter(j, aRow));
                                else
                                    if (op == _OR)
                                      return (previousRes || newApplyFilter(j, aRow));
                                  else
                                        if (op == _AND_)
                                          return (previousRes && newApplyFilter(j, aRow));
                                      else
                                            if (op == OR_)
                                              return (previousRes || newApplyFilter(j, aRow));
          }
          previousRes = res;
          op = c.operande;
        }
        return res;
    }

That's the point where my brain just signed off, and I asked the poor guy who inherited the code to "remove the whole sh***".

I didn't know that the guy just discovered the power of regular expressions and was ready to unleash it, even for this rather simple task. Here is the parsing code he commited:

private static  final Pattern  patternOperator = Pattern.compile("OR|AND");
static public Boolean getOperande(String aOp)
{
        Matcher matcherFilter  = patternOperator.matcher(aOp);
        if (matcherFilter != null && matcherFilter.matches())
        {
          String strOperator = matcherFilter.group(0);
               return ((strOperator != null) && (strOperator.equals("AND"))) ? AND
                                  : OR;
        }
        return OR;
}

Yep, the eight state boolean is still the worse piece of code I ever met. Replacement code is just a minor WTF in comparison.