[Okta Webinar] Learn how to a build a cloud-first strategyRegister Now

x
  • Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 297
  • Last Modified:

Feedback on effectiveness and efficiency of code

Hello there,

I have this class which is implemented on the server side. I want to get help from experts and get their feedback/suggestions/recommendations on this piece of code I have written and tell me if it has been written with proper standard. specially the hibernate section where I am querying the database. I will appreciate your help. I have attached my code.

cheers
Zolf
zz.txt
0
zolf
Asked:
zolf
  • 3
  • 2
  • 2
4 Solutions
 
Michel PlungjanIT ExpertCommented:
You for sure need to look at StringBuilder instead of + concatenation

This is also bad

if (dsRequest.getFieldValue("queryBatchNo") != null && "1".equals(dsRequest.getFieldValue("queryBatchNo").toString()))

instead save as string and test it twice
0
 
zolfAuthor Commented:
mplungjan

thanks for your feedbacks. can you please tell me more about this comment of yours. I did not understand what you meant by test it twice
instead save as string and test it twice

regarding the concatenation did you mean my sql queries
0
 
Michel PlungjanIT ExpertCommented:
yes I meant the queries


String queryBatchNo =  dsRequest.getFieldValue("queryBatchNo");

if (queryBatchNo!= null && "1".equals(queryBatchNo.toString()) )...

PS: There are many more considerations that Java experts can point out in your code. You have too many if statements and too big pieces of code inside each
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
girionisCommented:
A few things you could improve on:

1) Always use a package.
2) Don't use a System.out.println (use a logger instead)
3) Do not instantiate Long objects with new Long. Use Long.valueOf instead (it is more memory friendly).
4) Don't throw Exception, instead throw specific exceptions.
5) Avoid printStackTrace, again, use a logger.
6) Your singleton is not thread-safe. If you use this class in a multi-threaded environment you might end up with more than one instance of you class.
0
 
zolfAuthor Commented:
thanks you all for your comments

by: girionis

Your singleton is not thread-safe.

can you please tell me which code you meant.actually I will be using it in a multi-threaded environment and that was my concern and the reason for posting my code here to get experts like you all to help me improve my code.
0
 
girionisCommented:
I am talking about this bit of code

public static SupplierOrderCompleteBusinessLogic getInstance()
	{
		if (instance == null)
			instance = new SupplierOrderCompleteBusinessLogic();
		return instance;
	}

Open in new window

this is not thread safe (and as a side note, people can always create more instances since you provide an implicit default constructor). You must synchronise the getInstance() method by either doing

public static synchronized SupplierOrderCompleteBusinessLogic getInstance()
	{
		if (instance == null)
			instance = new SupplierOrderCompleteBusinessLogic();
		return instance;
	}

Open in new window

or use some synchronisation right before the instance creation.

But I would go with the initialisation on demand holder idiom solution if you can go with lazy loading,

        private SupplierOrderCompleteBusinessLogic() {}

        private static class Holder {
             private static final SupplierOrderCompleteBusinessLogic INSTANCE = new SupplierOrderCompleteBusinessLogic();
        }

	public static SupplierOrderCompleteBusinessLogic getInstance() {
             return Holder .INSTANCE;
        }

Open in new window

0
 
zolfAuthor Commented:
cheers
0

Featured Post

Free Tool: SSL Checker

Scans your site and returns information about your SSL implementation and certificate. Helpful for debugging and validating your SSL configuration.

One of a set of tools we are providing to everyone as a way of saying thank you for being a part of the community.

  • 3
  • 2
  • 2
Tackle projects and never again get stuck behind a technical roadblock.
Join Now