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.