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?
13
-
8@GoranJovic I don't personally see a problem with this question. I would say that a junior developer having issues with a codebase is a common enough problem (I myself have had this problem) that it warrants a question, and it can be answered.– user7007Commented Jan 29, 2012 at 15:55
-
2For inexperienced people most non-trivial code bases are hard to understand. You also need to recognize that there are many "why"'s that are not captured in the code, and you need to ask about.– user1249Commented Jan 29, 2012 at 16:08
-
4@GoranJovic We cannot provide a concrete black/white, yes/no answer on the matter. That does not make the question unanswerable.– user7007Commented Jan 29, 2012 at 16:09
-
2I worked for a landscaper a long time ago. They didn't write unit tests either. It annoyed me to no end.– Mike BaranczakCommented Jan 29, 2012 at 18:19
-
4Voting to reopen - one question is explicitly about talking to a boss/superior about their code, while this one is about organizational problems and concerns. Although there might be similarities, they are two distinct problems. I feel that there might be a duplicate out there, but I didn't see it in my quick search, and I know the question that this is currently marked as a duplicate of is not it.– Thomas Owens ♦Commented Jan 29, 2012 at 21:00
|
Show 8 more comments
6 Answers
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.
answered Jan 29, 2012 at 15:42
-
9And I would add: Listen to what they say and research whether their answers make sense. Look for cop out answers like "this is the way everyone does it" or "its working why change it" etc... if it seems like they do not give you actual technical reasons, that is a big problem. But first... Do not put them on the defensive by accusing them of something. I have made that mistake before and it is hard to repair.– c_makerCommented Jan 29, 2012 at 15:51
-
1
-
1@jamesson no I didn't. And I did not post from my SE account either :).– Why123Commented Jan 29, 2012 at 16:05
-
I know the age of the code wasn't mentioned, but as a senior developer, I can tell you I would be embarrassed to look at my own code from 10+ years ago. If someone asked me to explain it, I'd probably also explain how it should have been done instead. I hope I'd also be open to learning something new if there's an even better way of doing it that I don't know about.– robCommented Feb 17, 2015 at 23:06
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.
-
thanks for your answer. But there are questions that I need to ask such as "Why is this method 600 lines long?" for which I don't think there is a good answer.– Why123Commented Jan 29, 2012 at 16:19
-
@Why123 I'm not saying a 600 line method is a good thing, in fact I would say that it is probably bad no matter how you look at it, but you should still ask anyway. You may be assigned a task of breaking it down, or you may find there is a real reason behind it. There may not be an answer other than "an inexperienced programmer wrote this", but ask. Seeing how not to do something is just as important as seeing how to do it properly.– user7007Commented Jan 29, 2012 at 16:23
-
@Why123 Are you exaggerating? or is there really a 600 lines method?– toto2Commented Jan 29, 2012 at 19:20
-
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.
-
1If you're working at a place where pointing out bad code "won't make you any friends", you need to a find a new job. I would say that being confrontational about it is definitely not a good idea, but asking why code is that way and pushing to make it better definitely is. Commented Jan 29, 2012 at 20:27
-
If you've only been in environments where that's been your experience, more power to you. As I say, I'm not a programmer. However, my father and (believe it or not) grandmother are. Their experience is what causes me to think and talk this way. None of my interactions with senior programmers in any organization has lead me to question that. Largely the reason I didn't go into it myself.– jamessonCommented Jan 29, 2012 at 21:17
-
You can either go along with it and work in a company that sucks (and eventually become one of them), or get out (and make more money since people who are competent are worth more). Commented Jan 30, 2012 at 1:23
-
Again - it works for you - great. Can't say the same for anyone I know personally.– jamessonCommented Jan 30, 2012 at 1:36
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.
answered Jan 29, 2012 at 16:52
-
I like your suggestion. But as I pointed out the code has not been written with testability in mind. I would be going from one nightmare to another.– Why123Commented Jan 30, 2012 at 0:45
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.
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).
answered Jan 29, 2012 at 18:11
