Java static methods can be a code smell

Definition of code smell (from Wikipedia):



“any symptom in the source code of a program that possibly indicates a deeper problem.”

In Java,
static methods allow you to execute code at a “class scope” as opposed to an instance scope like member methods. This means, they rely on class-level variables (if any), parameters passed to the static method, or any other globally accessible data. They are NOT object oriented. An object has state associated with it and can be manipulated only through methods that implement the object’s “behavior.” Static methods do not operate on state, they are not object oriented, and in fact they are procedural.

Is this bad?

No. Although Java is object oriented, there will be times when procedural-like programming in Java is necessary and/or preferred. The real power in any object-oriented language is the ability to closely implement a model of real-life systems in code (see my
post about object-oriented modeling). But even in the most hard-core object models, there will mostly likely be some glue code, or infrastructure code that will be implemented in a procedural style.

So if procedural-like programming in Java isn’t “that bad” and static methods are a form of procedural programming, are static methods bad?

Ahh… the answer isn’t as simple as a “yes”, or a “no” regardless of what you may read on other blogs But I could probably ramble on and on about why it’s really a decision that has to be made in context, so let’s keep this article focused by examining a set of statements that I encountered in
“How to Mock Static Methods” from Michael Minella’s blog:

“Something that has become a fundimental [sic] piece of the language (all you need to do is look at the Apache Commons project to see that) is so bad that it must be avoided at all costs in the name of testing. Gosling (or someone on his team) put it in the language for a reason and to avoid those uses solely because your toolset doesn’t support the testing of it is nonsense. Time to get a new toolset.”

First, I’d like to point out that just because something has become a fundamental piece of a language doesn’t mean it’s “good” or something that should be done. Take a look at
checked exceptions for reference. I recall EJB 1.x and 2.x becoming a “fundamental” piece of Java EE in the past, so look at that for reference too.

Second, although I do agree with Michael in theory that avoiding a particular language feature because your tools don’t support it is silly, his premise is focused on static methods. Avoiding static methods because your tooling doesn’t support them is NOT nonsense at all. In fact, the type of impedance caused by some of the good testing and/or mocking frameworks (of which
Mockito is my favorite
:) ) and static methods can be confidently identified as a code smell. This does not mean we should not do it, but we should exert extra effort to understand why we are doing it and explore alternatives if there are “deeper problems.”

There are at least two types of static methods that I would like to point out don’t usually show much impedance with testing/mocking frameworks. The first type is static methods used as utility methods, as those found in a lot of the
apache commons libraries, or your own internal commons libraries. These are usually routines that are supportive of a particular method’s objective, and mocking/stubbing them out of a unit test wouldn’t make sense. They are part of the implementation, and should be tested as such. The second type is static methods used in place of constructors as Joshua Bloch showed in his book
“Effective Java.” This use of static methods allows you to construct a new object using a method with a very descriptive name, among some other advantages. An offshoot of this second type of static method could include factory methods, but that would depend on context.

The most glaring code smell as a result of static methods and testing-framework impedance arises when a unit relies on a static method that performs logic outside the scope of the responsibility of that unit. In these cases, your testing framework will fight you because you cannot stub/mock the out-of-scope logic because it is “hardcoded” via a static method. This can be considered a “deeper problem” and is the focus of most blogs that tell you not to use static methods because testing becomes abnormally difficult or impossible. Changing the design approach to follow the Dependency Inversion Principle is one alternative. A better understanding of how to test the unit is yet another.

I assert strongly that in the case of static methods, the push-back you may get from your testing frameworks is indicative of a code smell, not that you need to try and find a framework that uses complex trickery with class loader remapping as a solution. One should be poised to evaluate the use and fundamental drawbacks of a particular approach in their design. Michael’s blog entry lends the reader to too easily assume a new tool/framework just because Java supports static methods and your current testing frameworks illuminate an impedance — in this case, the impedance reflects a code smell and some deeper, more critical thinking is required.


Reference:
Java static methods can be a code smell from our
JCG partner Christian Posta at the
Christian Posta Software blog.

Source : http://www.javacodegeeks.com/2012/05/java-static-methods-can-be-code-smell.html

Advertisements

Programs and Technical Debt

Once you have a program (a collection of interrelated projects focused on one business goal) and you have technical debt, you have a much bigger problem. Not just because the technical debt is likely bigger. Not just because you have more people. But because you also geographically distributed teams, and those teams are almost always separated by function and time zone.

So, my nice example of a collocated team in
Thoughts on Infrastructure, Technical Debt, and Automated Test Framework, rarely occurs in a program, unless you have cross-functional teams collocated in a program. If they do, great. You know what to do.

But let’s assume you don’t have them. Let’s assume you have what I see in my consulting practice: an architecture group in one location, or an architect in one location and architects around the world; developers and “their” testers in multiple time zones; product owners separated from their developers and testers. The program is called agile because the program is working in iterations. And, because it’s a program, the software pre-existed the existence of the agile transition in the organization, so you have legacy technical debt up the wazoo (the technical term). What do you do?

Let’s walk through an example, and see how it might work. Here’s a story which is a composite from several clients; no clients were harmed in the telling of this story.

Let’s also assume you are working on release 5.0 of a custom email client. Release 4 was the previous release. Release 4 had trouble. It was late by 6 months and quite buggy. Someone sold agile as the way to make software bug-free and on-time.

You do not have automated tests for much of the code, unit tests or system tests. You have a list of defects that make Jack the Ripper’s list of killings look like child’s play. But agile is your silver bullet.

The program manager is based in London. The testers for the entire program are in Bangalore because management had previously fired all the testers and outsourced the testers. That was back in release 2. They have since hired all the Bangalore testers as employees of the Bangalore subsidiary. The program architect is based in San Francisco, and there is an architect team that is dispersed into 4 other teams: Denver, LA, Munich, and Paris. The developers are clustered in “Development Centers of Excellence:” Denver, LA, Cambridge, Paris, London, Munich, and Milan. That’s 8 development teams.

Oh, and if you think I’m kidding with this scenario, I’m not. This is what most of my clients with geographically distributed teams and programs face on a daily basis. They deserve your sympathy and empathy. Do not tell them, “Don’t go agile.” That’s nuts. They have a right to go agile. You can tell them, “Don’t go Scrum.” That’s reasonable. Scrum is for a cross-functional co-located team. Agile is for everyone. Scrum is for a specific subset.

What do you do?
  1. Assign specific testers to specific development teams. No calling people resources; that allows managers to treat people like resources and plug-and-play them. You need to get rock-solid teams together. Once you have teams together, you can name them.
  2. Name teams so the teams reflect the feature groups they work on. What does an email product do? It gets email, it sorts email, it deletes, it forwards, it creates new mailboxes, and so on. The eight feature teams had to be named for the feature areas: Platform for the general features, Sort, Delete, Forward. There were two teams who worked on Platform. They were called Platform 1 and Platform 2. At one point, someone suggested they call themselves Thing1 and Thing2 from the Dr. Seuss book.
  3. Make sure you have enough product owners so they can develop roadmaps for each feature area. With a roadmap, the teams know where they are going. Even more importantly, the architects know where the program is going.
  4. Architects think and provide just enough guidance ahead. In a small project, the architecture can probably evolve with the project. In a larger program, that risk is too large. You have too many people developing in parallel for the architecture to evolve on its own with no guidance. But I do not mean there should a Master Architect Who Knows All Handing Down the Architecture From On High. NO NO NO.
    I want the architect who is a working member of the development team, who also is part of an architecture community of practice team, who curates the architecture, who guides the business value of the architecture. I do not want Big Architecture Up Front. But Thinking Up Front? Sure, that’s a great idea. Stuck on only one idea? Bad. Willing to spike an idea? Great. Willing to play in a sandbox and debate several ideas? Great. I wrote about this before, in
    How Agile Architects Lead.
  5. Decide what done means for every feature. You must have acceptance criteria for each feature. What does that mean? You need a product owner present for each team. You still need the conversations with each team to discuss what done means. Especially with a geographically distributed team, you need the conversation when you create the backlog at the beginning of the iteration.
  6. The US development teams had trouble planning their iterations with their testers, because of the time zone differences with the testers. So, they asked their product owners if the product owner would write more than just a few phrases on the cards, because that would help them get through the iteration planning meeting faster. Someone was going to get up early or stay up late, and either way, someone was going to suffer. It made more sense to have a little bit more preparation than less sleep.
  7. Decide to do continuous integration and stick with it. Especially if you know you have technical debt and you don’t want to create more, you have to do continuous integration now. That prevents more technical debt.
  8. I have recommended to some teams that they have one-week iterations so that they stop the estimation nonsense and make their stories small. The point of estimation is so that you have an idea of what you can do as a team and not commit to more than that. The idea is that if you know what it takes to make your stories small, you will.
    Instead, we have all these crazy rituals around estimation and management tracking velocity of all things. (Yes, I’ve been drafting this post for a long time, and I wrote
    Why Does Management Care About Velocity? last week.) You know, velocity is a little like weight. Only you and your doctor need to know your weight. If you are healthy, you are fine. If you are not, you need to change something.If your team velocity is not healthy, you, as a team, need to change it. But, your management has no business butting its head in. Only you can change it.
  9. When you limit the iteration length, you tend to have the team swarm around a story. This is a tendency, not a given. If I really was the Empress of the Universe, I would decree this, but I’m not, so I won’t. If you want to decrease technical debt, or even eliminate it on your program, explain that your team will only work on one story at a time until that story is done. That story will be polished and gleaming. Fast. You will not have to worry about what kinds of testing will be done. All if it will be done.
  10. Explicitly discuss what you will automate for testing and when. In a program, I assume we will have automated system tests first. I assume we will do exploratory tests later. That’s because if you don’t start building something for test automation when you start the program and refactor as you proceed, you can never catch up. I assume every time we fix a defect, we will have an automated test for it. I also assume we build these assumptions into how we develop 🙂

So far, this is all about preventing more technical debt, not what happens when you trip over technical debt as you enter code or tests you never looked at before.

If you expected to walk into a closet, take out a shirt, and close the closet door, that’s one thing. But now, you stepped into something out of one of those death-by-hoarding shows on TV, you have an obligation to do something. You can document the problem as you encounter it; you can let the product owner know; file a defect report; write a test so you can contain the debt; and maybe you have more options. Whatever you do, make sure you have done something. Do not open the door, see the mess inside and close the door on the mess. It’s tempting. Oh my, it is tempting.

See, on programs because of the size, everything is magnified. With more people and more teams, everything is harder. Things happen faster. If you have co-located cross-functional teams, no problem. But if you don’t have co-located cross-functional teams, you have to work with what you have. And, if you already have a big legacy product, you want to address technical debt in small chunks, refactoring in small bits, integrating as you proceed.

My philosophy is this: the bigger the program, the more you need to become accustomed to working in small chunks, integrating as you go. Fully implement a small story, integrate it on the mainline. Everyone on the program does that. If you need help from an integration team, so be it.

But, if everyone only implements small stories, and everyone takes care of their own technical debt as they discover it, you don’t need an army of integration people. You only need an army of integration people when you have technical debt around integration and release. Fix that, and everyone can become responsible for their own integration.

And, if you can’t release, that’s where the architects should start. If you can’t do continuous integration, that’s where the architects should start. Because that’s what’s preventing you from making progress on the product. Work backwards from release, and then the architects can work on the rest of the product. Until you can release and build reliably, the rest of the product doesn’t matter.


Reference:
Programs and Technical Debt from our
JCG partner Johanna Rothman at the
Managing Product Development blog.

Source : http://www.javacodegeeks.com/2012/05/programs-and-technical-debt.html

Code comments gone wrong

Adding code comments is supposed to be good practice, but here is why it often fails:
  • Code is the single authoritative source of truth in a program!
  • There is no way to ensure that code comments are correct at all times (not always updated as code changes).
  • Comments are written in human language which can be prone to misinterpretation.
First put your good intentions into writing simple and readable code.


Write self descriptive code ! Your code should be read like sentences. Avoid smart shortcuts and tricks because they break the reading. Expect the reader to have solid programming knowledge but no knowledge about the purpose of your code. If code is too compact add extra code steps to document it, for example:
...
final Person dummyPerson = new Person("Joe", "Bloggs");
return dummyPerson;
Instead of using a comment:
...
// Return dummy person.
return new Person("Joe", "Blogs");
Ok ok, this example was a bit silly but you got the idea.

Using long names is considered bad practice, I disagree. Prefer using long explicit names over short meaningless names which require code comments. Sometimes long names are really annoying, for example when they keep appearing everywhere in some algorithm, in that case you could use a comment.

Consider using more columns:

The default max of 80 columns is terrible, use a wide screen and use 120 columns or more, the code will be more readable because long lines will not wrap anymore and you can use longer more explicit names.

Use assertions to document pre and post conditions instead of lengthy comments.
public List<String> listFiles(final String folderUrl) {
assert folderUrl!= null;
assert folderUrl.endsWith("/");
  ...
}
If you write an API a good documentation is necessary but for internal code I think comments should not replace good naming and code clarity. I use code comments when the code is not really self documenting. Comments should convey what code cannot. They should explain the reasons for a specific design decision, they should explain what code is supposed to achieve and why.

Learn how to use the Javadoc, it not only looks better, it can also help automatically update some documentation. When referring to code try using the link tag. Your IDE may automatically update the linked method and class names during renaming which ensures that some of your documentation stays up to date.
/**
* Use the link tag: {@link SomeClass#someMethod}
*/
Links:


Reference:
Code comments gone wrong from our
JCG partner Christophe Roussy at the
Javarizon blog.

Source : http://www.javacodegeeks.com/2012/05/code-comments-gone-wrong.html

Building security into a development team

Getting application developers to understand and take responsibility for software security is difficult. Bootstrapping an Appsec program requires that you get the team up to speed quickly on security risks and what problems they need to look for, how to find and fix and prevent these problems, what tools to use, and convince them that they need to take security seriously. One way to do this is to train everyone on the development team on software security.

But at RSA 2011, Caleb Sima’s presentation
Don’t Teach Developers Security challenged the idea that training application developers on software security will make a meaningful difference. He points out (rightly) that you can’t teach most developers anything useful about secure software development in a few hours (which as much Appsec training as most developers will get anyways). At best training like this is a long-term investment that will only pay off with reinforcement and experience – the first step on a long road.

Most developers (he suggests as many as 90 out of 100) won’t take a strong interest in software security regardless. They are there to build stuff, that’s what they get paid for, that’s what they care about and that’s what they do best. Customers love them and managers (like me) love them too because they deliver, and that’s what we want them spending their time doing. We don’t want or need them to become AppSec experts. Only a few senior, experienced developers will “get” software security and understand or care about all of the details, and in most cases this is enough. The rest of the team can focus on
writing good defensive code and using the right frameworks and libraries properly.

Caleb Sima recommends starting an Appsec program by working with QA. Get an application security assessment: a pen test or a scan to identify security vulnerabilities in the app. Identify the top 2 security issues found. Then train the test team on these issues, what they look like, how to test for them, what tools to use. It’s not practical to expect a software tester to become a pen testing expert, but they can definitely learn how to effectively test for specific security issues. When they find security problems they enter them as bugs like any other bug, and then it’s up to development to fix the bugs.

Get some wins this way first. Then extend security into the development team. Assign one person as a security controller for each application: a senior developer who understands the code and who has the technical skills and experience to take on security problems. Give them extra Appsec training and the chance to play a leadership role. It’s their job to assess technical risks for security issues. They decide on what tools the team will use to test for security problems, recommend libraries and frameworks for the team to use, and help the rest of the team to write secure code.

What worked for us

Looking back on what worked for our Appsec program, we learned similar lessons and took some of the same steps.

While we were still in startup, I asked one of our senior developers to run an internal security assessment and make sure that our app was built in a secure way. I gave him extra time to learn about secure development and Appsec, and gave him a chance to take on a leadership role for the team. When we brought expert consultants in to do additional assessments (a secure design review and code review and pen testing) he took the lead on working with them and made sure that he understood what they were doing and what they found and what we needed to do about it. He selected a static analysis tool and got people to use it. He ensured that our framework code was secure and used properly, and he reviewed the rest of the team’s code for security and reliability problems. Security wasn’t his entire job, but it was an important part of what he did. When he eventually left the team, another senior developer took on this role.

Most development teams have at least 1 developer who the rest of the team respects and looks to for help on how to use the language and platform correctly. Someone who cares about how to write good code and who is willing to help others with tough coding problems and troubleshooting. Who handles the heavy lifting on frameworks or performance engineering work. This is the developer that you need to take on your core security work. Someone who likes to learn about technical stuff and who picks new things up quickly, who understands and likes hard technical stuff (like crypto and session management), who makes sure that things get done right.

Without knowing it we ended up following a model similar to Adobe’s “
security ninja” program, although on a micro-scale. Most developers on the team are white belts or yellow belts with some training in secure software development and defensive programming. Our security lead is the black belt, with deeper technical experience and extra training and responsibility for leading software security for the application. Although we depended on external consultants for the initial assessments and to help us lay out a secure development roadmap, we have been able to take responsibility for secure development into the development team. Security is a part of what they do and how they design and build software today.

This model works and it scales. If as a manager you look at security as an important and fundamental technical problem that needs to be solved (rather than a pain-in-the-ass that needs to be gotten over), then you will find that your senior technical people will take it seriously. And if your best technical people take security seriously, then the rest of the team will too.



Source : http://www.javacodegeeks.com/2012/05/building-security-into-development-team.html