• Post Reply Bookmark Topic Watch Topic
  • New Topic

Refactoring Help  RSS feed

 
Connor Crofton
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hey guys.

I'm new to this forum and I need some help refactoring this Java code. I'm new to both Java and refactoring and I'm supposed to do a Refactoring assignment this week for my Software Development Java Class.

Here's the code (which I typed in on the java program Eclipse):



As you already know, refactoring means changing a code to make it easier and readable for programmers. Good code standards include:
• No classes over 1000 lines (breakup large classes and extract superclasses)
• No methods over 50 lines (breakup long methods and extract methods)
• No duplicate code
• Meaningful comments
• Meaningful method names (findRecordById () is clearer than findMe1())
• Meaningful variable names, not misleading, cryptic or cute
• No repeating variable names inside and outside methods
• No complex conditional logic. Use Polymorphism instead of conditionals where appropriate
• Clear purpose of each class and method (no dual purposes)
• Clear logic of each class and method

Any help would be best.
 
Fred Kleinschmidt
Bartender
Posts: 571
9
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What part of that code do you think needs refactoring?
 
Connor Crofton
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think somewhere in the static void main and static double sum sections of the code, Fred.
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you take those standards you were given and look for anything that violates them, that's what's called finding "smells" -- a smell is a violation of a rule that makes the program design "good".  I don't necessarily agree with all the rules you were given but if that's what you were given as guidelines, then that's what you should base your judgement of what is "smelly" in your code.  So, given that, what do you find smelly in that code?
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The rules I most agree with are :

- Meaningful variable names, not misleading, cryptic or cute
- Clear purpose of each class and method (no dual purposes)
- Clear logic of each class and method

There's a subtle nuance to the "no duplicate code" rule. This is also know as the DRY (Don't Repeat Yourself) Principle. The nuance is that DRY is not about mechanical, character-for-character duplication in code. It's about a duplication of knowledge/ideas. Sometimes these two are the same but sometimes they're not.

The other rule that I like to follow which isn't given in that list is to follow the Single Level of Abstraction Principle (SLAP)
 
Henry Wong
author
Sheriff
Posts: 23295
125
C++ Chrome Eclipse IDE Firefox Browser Java jQuery Linux VI Editor Windows
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Connor Crofton wrote:
As you already know, refactoring means changing a code to make it easier and readable for programmers.


It's two methods. Five lines of code each. No objects directly being instantiated. I would seriously doubt that there are any professional programmers who can't read that...

I guess you can add a couple of comments, but even then, the code is ridiculously simple. Even in a code review, and I was being really strict, I am not sure if I would even mention the lack of comments.

Henry
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Even with one or two lines of code, the usual suspects can almost always be found: Poorly chosen names and unclear intent.

One glance already shows a violation of this guideline: Prefer programming to abstractions rather than implementations
 
Connor Crofton
Greenhorn
Posts: 5
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Henry Wong wrote:
Connor Crofton wrote:
As you already know, refactoring means changing a code to make it easier and readable for programmers.


It's two methods. Five lines of code each. No objects directly being instantiated. I would seriously doubt that there are any professional programmers who can't read that...

I guess you can add a couple of comments, but even then, the code is ridiculously simple. Even in a code review, and I was being really strict, I am not sure if I would even mention the lack of comments.

Henry


If I were to change my code to just five lines, what would it look like Henry?
 
Junilu Lacar
Sheriff
Posts: 11494
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Connor Crofton wrote:If I were to change my code to just five lines, what would it look like Henry?

Looks like we forgot to formally welcome you to the Ranch, so...

Welcome to the Ranch!

We don't do people's homework for them here so you're going to have to figure that one out yourself. If you were going to change just 5 lines of code, which ones would they be, per your guidelines?
 
Liutauras Vilda
Sheriff
Posts: 4923
334
BSD
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Connor,

Welcome again. Now let's start by eliminating what you have so far:
  • No classes over 1000 lines - Do you have such long code here? Probably not, that means not much you can improve on that point.
  • No methods over 50 lines - Is this a case here? Probably similarly as above, nope.
  • No duplicate code - ? Do you see much code repetition here? Probably again, not really.


  • Meaningful comments - Wuolia, you don't have comments yet. Be careful here, don't start by desperately writing
  • i.e.:
    That would be a bit pointless, but not knowing your teacher/-s expectations, I'd say refer to class slides, examples and try to spot comments style patterns.

  • Meaningful method/variables names - Do you think you could improve something here? I could I think. At least in 4-5 places.
  • No repeating variable names inside and outside methods - I don't really get this, what is meant here, even if my thinking is correct, not sure I fully agree with that. At least not in all scenarios. Apart from that, provided code violates this point from what I can see.
  • Clear purpose of each class and method - MyClass, is it clear purpose of it? Let's do this way, I just wrote small program on my local machine and namaed one of its class as LiutaurasVildaClass, can you please tell me what it does?


  • I think you could get an idea where you possibly can start. At least this is where I'd start. Actually first I'd improve formatting as it is inconsistent as well as indentation. Missing braces around loop statement. Unecessary empty lines in one place, while in other places there are no.
     
    Campbell Ritchie
    Marshal
    Posts: 56570
    172
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Liutauras Vilda wrote:. . .
  • Meaningful comments - Wuolia, you don't have comments yet. Be careful here, don't start by desperately writing
  • i.e.:
    . . .
    I can think of even better than thatAll readers will know implicitly why you want to start counting from 3 rather than 0 or 1.

    Those comments are both wrong style: they shou‍ld be /* comments */ Reserve // comments for something after the end of the line (or commenting‑out). Also make sure any comments maintain correct line lengths.

    And welcome to the Ranch :) again.
     
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!