11

I recently joined a company and it is my first job. When reading the code base, I felt that the code was not well written. It seemed to me that the code had most of the problems mentioned here and also seemed to have an Anemic Domain Model. There are no unit tests and they don't employ any code quality checking tools like findbugs or pmd. The problem I have is that the code is very difficult to understand. Maybe my conclusions are wrong because I am not that experienced. I need advice on whether to communicate the above facts to a superior or not. If I am to communicate, to whom(Tech Lead, Architect, Product Manager) and how? And if I do communicate will they take it badly since I'm a Junior and has no experience?

Why123
  • 121

6 Answers6

29

Ask any senior developer on your team. Don't tell them you think it's wrong. Instead, Tell them you don't understand the code and are curious why it is developed the way that it is. Ask them to explain the code to you.

Bryan Oakley
  • 25,479
9

Asking for an explanation of the code in a non-confrontational manner from a senior developer or someone who knows enough about the structure and details of the code is a good idea.

Make sure to ask about the details (why the code is the way it is) you are questioning and make note of what the answer is. If after the explanation is given and you are not satisfied, dig into the problem a bit more and create a case and be prepared to back it up. Present it to whomever helped you first and ask them what they think of your findings and work from there.

Depending on your culture of your workplace, you may wind up waiting for an opportunity to have someone go over the code with you. Take this time to look into the issue more if at all possible. A junior developer is generally not expected to be very experienced and situations like this will arise.

3

In reality, the answer is "no". Words like "good" or "bad" are only used by management. To define what is "good" you must become management. Whether or not this is what you want is your preference.

I am not myself a programmer (well, not by job secription anyhow), but I work with/know many programmers. I know a woman, for instance, who is low-level management for [insert majeor telecom company] and man who is a senior design engineer for [insert major audiovisual co]. The woman is always going on about how it's "insulting" and "unreasonable" for a person of her carreer level and experience to be expected to learn new languages. The man started doing ARM development but doesn't want to use "unfinished" software like eclipse so he's always bugging me to pirate paid tools. Moral of the story - bad code exists for all kinds of reasons. Pointing it out won't make you any friends.

That said, if you really do want to do this the other advice presented here is really the way to go.

jamesson
  • 537
3

In addition to what @Bryan Oakley says, I suspect the only explanation for the lack of unit tests is that the team never feels they have the time to write them, This is a common dilemma, and it also makes refactoring extremely difficult.

So, I'd suggest that the most constructive course you could take is to volunteer to write unit tests for the code--perhaps as a way to "get a better understanding" of it. This could solve one of the major smells you've encountered while opening the door to fix others.

Matthew Flynn
  • 13,495
  • 2
  • 39
  • 58
2

Don't be afraid to question anything, just don't start pointing fingers. For all you know, you could be experiencing a sever case of Expert Beginners, and people may have just become set in their ways. In which case, you could always try to enlighten them. But, developer egos aren't easy to deal with.

This also may be legacy code that was written before the time of the current developers. They may be just as disgruntled over it as you are.

BrandonV
  • 1,356
0

About the Anemic Domain Model. I too was confused about objects that are used like structs with getter/setters. I don't particuarly like how it is used for EVERYTHING especially in Java projects. There is a case for this design though.

A system may have multiple computers using multiple languages. The objects could be used by javascript in a browser, a C++ app serverside, a Java app serverside, some webservices implemented in Python on a different network, etc.

Some behaviors are different if you are on the server or the client. Consider a "save" method. If the object is in javascript it cannot use the exact same technique to get that data into the database that server-side code would. You could hide a service call in the javascripts "save" method. But its not bad for code to be "aware" of the tier it's on and call the service explicitly.

Some behaviors should not even exist depending on which tier you're on. Consider a "draw" method. This would only be relevant in the client-side javascript. Packaging behavior with data can violate the principals of separation. Drawing should only be done on the front end computers. Although you could give the object different methods on each tier, it doesn't feel right.

Instead you go with the old fashioned C-style approach. You have a struct-like object that you pass around between the computers. (multi-language, different networks, serverside, client side). The C-style methods are "services" and may be a web-service. You may be using some neat OOP polymorphism within each tier, but the apps communicate in the old C-style technique. (which is not always a bad thing).

Lord Tydus
  • 1,441