Keep the hierarchy of container classes simple, just a single level deep.

Bad:

:
// a map of list of file information strings, (looked up by filename String))
private Map parsedFiles = new HashMap(); // [String filename] => List => String

public synchronized Map getParsedFiles()
{
    return parsedFiles;
}
:

In this case, the developer has created a referential mess with a Map of Lists of Strings.  Worse, the synchronized does very little – the caller has full access to the parsedFiles map, including the ability to make changes when not threadsafe to do so.

Good:

:
private Map parsedFiles = new HashMap(); // [String filename] => FileData

public synchronized FileData getFileData(String filename)
{
    return (FileData)parsedFiles.get(filename);
}

public synchronized void addFileData(String filename, FileData fileData)
{
    parsedFiles.set(filename,fileData);
}
:

The developer has wrapped the List of Strings in a class.  Suddenly the data being represented is clear.   Better still, the only way to access parsedFiles is through synchronized helper methods getFileData( ) and setFileData( ) on the owning class, hiding the existence of parsedFiles completely.  The synchronized references allow multiple threads to make changes to parsedFiles without conflict.

blog comments powered by Disqus