Win a copy of Kotlin in Action this week in the Kotlin forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Code Enhancement Needed for a better design pattern  RSS feed

 
Jeansonne Pierre
Ranch Hand
Posts: 75
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Guys,

Kindly need your expertise here to enhanced my code logic in these 2 endpoint class as below,
because my amateur Java level now is stopping me to improve this code.

Question:
1) Seems like i'm using a lot of dummy logic/method. Can this code be made more elegant and with improved clarity?
2) Please correct any mistake that causing performance issue in this code because i feel like im not implementing any good standard design pattern in my code.


Thanks in advanced, your help is much appreciated.

Abstract Class


Endpoint A


Endpoint B
 
Winston Gutkowski
Bartender
Posts: 10573
65
Eclipse IDE Hibernate Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jeansonne Pierre wrote:
1) Seems like i'm using a lot of dummy logic/method. Can this code be made more elegant and with improved clarity?
2) Please correct any mistake that causing performance issue in this code because i feel like im not implementing any good standard design pattern in my code.

A few things I see:
  • There's almost never a need to use a trinary operator to return a boolean; and certainly not in an if statement.
    if((newType.equalsIgnoreCase(UtilConstant.CONTACTDNC_TYPE_DNC)) ? true : false)
    is the same thing as
    if((newType.equalsIgnoreCase(UtilConstant.CONTACTDNC_TYPE_DNC))
  • Your names are very long-winded.
  • You use two different forms to check for constants: if...else and a switch statement in the same place (lines 27-61 of Endpoint A).
    Personally, I prefer the latter; but whatever you choose, be consistent.
  • You make your code quite tortuous just to have a single return statement (at least I assume that's why you do it).
  • What about:Then it's possible that you don't have to declare 'response' in your superclass (which breaks encapsulation anyway).

    I hate to say, but it looks like the sort of code that code generators tend to spew out - verbose and monolithic - but at least they have the excuse that theirs is generally not for human consumption. I presume yours is.

    Sorry if it sounds harsh, but you did ask. :-)

    HIH

    Winston
     
    Jeansonne Pierre
    Ranch Hand
    Posts: 75
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Winston Gutkowski,

    Thanks a lot for your valuable time and help. I'm sincerely appreciate your valuable guidance and advice.
    I have correct my code according to your suggestion and i've learned something from you again. The code looks much better now.
    Thanks a lot Winston Gutkowski. Thanks

    Feel free to give feedback on my code if you think there's still changes/modification needed. I'll more than happy to learn from my mistake.
    Again thanks a lot, Winston Gutkowski.
     
    Winston Gutkowski
    Bartender
    Posts: 10573
    65
    Eclipse IDE Hibernate Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Jeansonne Pierre wrote:Feel free to give feedback on my code if you think there's still changes/modification needed. I'll more than happy to learn from my mistake.

    OK, well there are a few more things I see:

    1. Don't scrunch up all your code. Adding whitespace makes it much more readable.

    2. On a similar tack: Don't try to do too much in a single statement. Breaking things up makes them more readable, and also helps if you need to debug.

    3. You seem to repeat an awful lot of JSON boilerplate. I suspect that this is because your JSON code is too closely coupled with your information code but, not being a JSON expert, I don't know whether it's necessary or not.
    It does bother me a bit that you create a new deserializer every time you call one of your methods though. At the very least, I would have thought you could add something like:
    to your superclass, and then use:
    VoidContactRequest voidContactRequest = getVCRequest(request);
    in your subclass methods.
    That way, if there IS a better way of handling the deserializer, you only have to change the code in one place.

    4. Whenever you have a big list of static constants, consider using an enum. For example:
    then your buildNote() code becomes something like:
    Enums are incredibly useful things, and it's well worth reading up about them.

    HIH

    Winston
     
    Jeansonne Pierre
    Ranch Hand
    Posts: 75
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Hi Winston Gutkowski,

    Wao, this is impressive for me. your advice really makes my code more elegant and with improved clarity and more robust.
    Really appreciate your help. I will never have this design if you didn't told me. Thanks a lot. This is exactly what i'm looking for. Thanks again.

    I have change my ContactAbstractController class as below:

     
    Winston Gutkowski
    Bartender
    Posts: 10573
    65
    Eclipse IDE Hibernate Ubuntu
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Jeansonne Pierre wrote:Really appreciate your help. I will never have this design if you didn't told me. Thanks a lot. This is exactly what i'm looking for...

    You're most welcome.

    One of the traps that most beginner (or "out of practise") programmers fall into is that they equate programming with coding. I think you were starting to realise this when you began this thread, because you had a pile of code that lacked cohesion. Coding is a mechanical process that simply involves listing the steps required to solve a problem, whereas design involves stepping back and looking at a problem as a whole, and trying to work out the tools or approaches that will help to solve it.

    The trouble is: coding is more satisfying. It's relatively simple, and we feel like we're "getting something done". For small problems, it's also quite possible to translate straight to code with decent results, but it doesn't scale well as a methodology. And IMO, the two processes are almost mutually exclusive: You can't do one while you're doing the other.

    So my advice:
  • Resist the urge to code.
  • Before you start to code, think about what you want to do, and the classes (or enums) you need to do it. You might find some help in the WhatNotHow (←click) page.
  • When you get stuck: StopCoding (←click).
  • If you think you're "missing" something, STOP, and work out if you might need a new class. Beginners, in general, don't write nearly enough classes.

  • I like to think of a computer program as a house or a landscape garden. Both need workers and carpenters and plumbers to fit things together, but they also need an architect - ie, someone with the "vision" of what the thing will look like (or do) when it's complete. And an architect doesn't deal with piping or plumbing; not because he (or she) can't do it, but because it's a distraction. Similarly, a carpenter or joiner can't solve design problems.

    Hope it helps; and good luck.

    Winston
     
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!