3
\$\begingroup\$

Is it right? any improvements? different approach...?

import java.util.Properties;

import javax.ejb.EJBException;
import javax.ejb.MessageDrivenBean;
import javax.ejb.MessageDrivenContext;
import javax.jms.JMSException;
import javax.jms.Message;
import javax.jms.MessageListener;
import javax.jms.Queue;
import javax.jms.QueueConnection;
import javax.jms.QueueConnectionFactory;
import javax.jms.QueueSender;
import javax.jms.QueueSession;
import javax.jms.Session;
import  javax.jms.TextMessage;
import javax.naming.Context;
import javax.naming.InitialContext;
import javax.naming.NamingException;


public class CalculatorImpl implements MessageDrivenBean,MessageListener {

    private static Properties properties;
    private static Context jndiContext;
    private static final String
        //JNDI_PROVIDER_CONTEXT_FACT    = "weblogic.jndi.WLInitialContextFactory" //weblogic specific value
          JNDI_PROVIDER_CONTEXT_FACT    = "org.jnp.interfaces.NamingContextFactory";
    ;
    private static final String
        //JNDI_PROVIDER_URL = "t3://localhost:7003"//weblogic specific value
        JNDI_PROVIDER_URL   = "jnp://localhost:7003"//Mention valid provider url
    ;
    private static final String
        JNDI_SECURITY_PRINCIPAL = "weblogic"//Mention valid principal value
    ;
    private static final String
        JNDI_SECURITY_CREDENTIALS   = "weblogic12"//Mention valid credential value
    ;

    private String splitMsg,expr1,expr2     = null;
    private double trigVal                  = 0.0;
    private boolean negPostNum              = false;
    private int subIdx,addIdx,postIdx,preIdx = 0;

    //----------------------------------------------------------------------------------------  
    public CalculatorImpl(){

        properties = new Properties();
        properties.put (Context.PROVIDER_URL, JNDI_PROVIDER_URL);
        properties.put(Context.INITIAL_CONTEXT_FACTORY,JNDI_PROVIDER_CONTEXT_FACT);
        properties.put(Context.SECURITY_PRINCIPAL, JNDI_SECURITY_PRINCIPAL);
        properties.put(Context.SECURITY_CREDENTIALS, JNDI_SECURITY_CREDENTIALS);

    }
    //----------------------------------------------------------------------------------------
    public void onMessage(Message p_message){
        Message                 l_message           =   p_message;
        String                  l_requestStr        =   null,
                                l_correlationID     =   null,
                                l_result            =   null;
        try {

            l_correlationID = l_message.getJMSCorrelationID();
            if(l_correlationID == null){
                l_correlationID = l_message.getJMSMessageID();
            }
            if (l_message instanceof TextMessage) {
                l_requestStr    = ((TextMessage) l_message).getText ();
            } else {
                throw new Exception (
                    "Message Type Not Supported : " 
                +   l_message.getClass ().getName ()
                );
            }
            l_result = calculate(l_requestStr);
            writeResponse(l_result,l_correlationID);

        } catch (EJBException e){
            e.printStackTrace();
            throw new EJBException();
        } catch(JMSException jex){
            jex.printStackTrace();
            throw new EJBException();
        } catch(Exception ex){
            ex.printStackTrace();
            try{
                writeResponse(ex.getMessage(),l_correlationID);
            } catch(Exception e1){
                e1.printStackTrace();
            }
        }
    }

    //----------------------------------------------------------------------------------------
    /*The calculate function takes a string parameter,fetches the constant, operator 
    and function & returns the calculated value.*/ 

        public synchronized String calculate(String p_msg) throws Exception {
            String l_message        =   p_msg;
            try
            {
                l_message=l_message.replace(" ", "");
                System.out.println("CalculatorTest.calculate() "+l_message.intern());
                while(l_message.indexOf("sin(") != -1){
                    l_message = getSin(l_message);
                    //System.out.println("CalculatorTest.calculate() aftersin "+l_message);
                }
                while(l_message.indexOf("cos(") != -1){
                    l_message = getCos(l_message);
                    //System.out.println("CalculatorTest.calculate() aftercos "+l_message);
                }
                while(l_message.indexOf("(") != -1){
                    l_message = getBrac(l_message);
                    //System.out.println("CalculatorTest.calculate()afterbrac "+l_message);
                }
                while(l_message.indexOf("/") != -1){
                    l_message = getDiv(l_message);
                    //System.out.println("CalculatorTest.calculate()afterdiv "+l_message);
                }
                while(l_message.indexOf("*") != -1){
                    l_message = getMult(l_message);
                    //System.out.println("CalculatorTest.calculate()aftermult "+l_message);
                }
                while(l_message.indexOf("+") != -1){
                    l_message = getAdd(l_message);
                    //System.out.println("CalculatorTest.calculate()afteradd "+l_message);
                }
                while(l_message.indexOf("-") != -1 && l_message.indexOf("Terminate")==-1){
                    l_message = getSub(l_message);
                    //System.out.println("CalculatorTest.calculate()aftersub "+l_message);
                } 
                if(l_message.indexOf("Terminate")!=-1){
                    l_message = l_message.substring(0, l_message.indexOf("Terminate"));
                }

                System.out.println("CalculatorTest.calculate() result "+l_message);

            } catch (Exception e){
                e.printStackTrace();
                throw new Exception("Error Occured During Calculation for "+p_msg);
            } finally {
                splitMsg= null; expr1= null;expr2= null;
                trigVal                     = 0.0;
                negPostNum                  = false;
                subIdx = 0;addIdx = 0;postIdx = 0;preIdx = 0;
            }

            return l_message;
        }
        //----------------------------------------------------------------------------------------

        //This method returns the sin value for the double parameter passed
        public String getSin(String p_message) throws Exception
        {
            splitMsg=expr1=expr2 = null;
            trigVal              = 0.0;

            splitMsg=p_message.substring(p_message.indexOf("sin("),p_message.length());
            splitMsg=splitMsg.substring(0,splitMsg.indexOf(")")+1);
            trigVal=Double.parseDouble(splitMsg.substring(splitMsg.indexOf("(")+1,splitMsg.indexOf(")")));
            p_message = p_message.replace(splitMsg,String.valueOf(Math.sin(trigVal)));
            return p_message;
        }
        //----------------------------------------------------------------------------------------  
        //This method returns the cos value for the double parameter passed
        public String getCos(String p_message) throws Exception
        {
            splitMsg=expr1=expr2 = null;
            trigVal              = 0.0;

            splitMsg=p_message.substring(p_message.indexOf("cos("),p_message.length());
            splitMsg=splitMsg.substring(0,splitMsg.indexOf(")")+1);
            trigVal=Double.parseDouble(splitMsg.substring(splitMsg.indexOf("(")+1,splitMsg.indexOf(")")));
            p_message = p_message.replace(splitMsg,String.valueOf(Math.cos(trigVal)));
            return p_message;
        }
        //----------------------------------------------------------------------------------------
        //This method returns the value of expression within brackets
        public String getBrac(String p_message) throws Exception
        {
            String l_bracMsg        = null;
            splitMsg=expr1=expr2    = null;

            splitMsg=p_message.substring(p_message.indexOf("("),p_message.length());
            l_bracMsg=splitMsg.substring(splitMsg.indexOf("("),splitMsg.indexOf(")")+1);;
            splitMsg=splitMsg.substring(1,splitMsg.indexOf(")"));

            while(splitMsg.indexOf("/") != -1){
                splitMsg = getDiv(splitMsg);
            }
            while(splitMsg.indexOf("*") != -1){
                splitMsg = getMult(splitMsg);
            }
            while(splitMsg.indexOf("+") != -1){
                splitMsg = getAdd(splitMsg);
            }
            while(splitMsg.indexOf("-") != -1){
                splitMsg = getSub(splitMsg);
            }
            p_message = p_message.replace(l_bracMsg,splitMsg);
            return p_message;
        }
        //----------------------------------------------------------------------------------------
        //This method returns the value of addition between two expressions

        public String getAdd(String p_message) throws Exception
        {
            splitMsg=expr1=expr2            = null;
            subIdx=addIdx=postIdx=preIdx    = 0;
            negPostNum                      =false;
            double addVal                   =0.0;

            expr2=p_message.substring(p_message.indexOf("+")+1,p_message.length()).trim();
            expr1=p_message.substring(0,p_message.indexOf("+")).trim();
            while(expr2.indexOf("-")!=-1 || expr2.indexOf("+")!=-1){
                subIdx=expr2.indexOf("-");
                addIdx =expr2.indexOf("+");
                postIdx=Math.min(addIdx==-1?99999999:addIdx, subIdx==-1?99999999:subIdx);

                if(postIdx ==0){
                    negPostNum = true;
                    expr2=expr2.substring(1,expr2.length()).trim();
                    continue;
                }
                if(postIdx == -1){
                    negPostNum = true;
                    expr2=expr2.substring(1,expr2.length()).trim(); 
                } else{
                    expr2=expr2.substring(0,postIdx).trim();
                }


            }
            if(expr1.indexOf("-")!=-1 || expr1.indexOf("+")!=-1){
                subIdx=expr1.lastIndexOf("-");
                addIdx =expr1.lastIndexOf("+");
                preIdx=Math.max(addIdx, subIdx);
                expr1=expr1.substring(preIdx+1,expr1.length()).trim();
            }
            if(negPostNum){
                splitMsg=expr1+"+-"+expr2;
                addVal = Double.parseDouble(expr1) + Double.parseDouble("-"+expr2);
            } else{
                splitMsg=expr1+"+"+expr2;
                addVal = Double.parseDouble(expr1) + Double.parseDouble(expr2);
            }
            return p_message = p_message.replace(splitMsg,String.valueOf(addVal));

        }
        //----------------------------------------------------------------------------------------
        //This method returns the value of substraction between two expressions

        public String getSub(String p_message) throws Exception
        {
            splitMsg=   expr1=expr2         = null;
            subIdx=addIdx=postIdx=preIdx    = 0;
            negPostNum                      = false;
            double subVal                   = 0.0;

            expr2=p_message.substring(p_message.indexOf("-")+1,p_message.length()).trim();
            expr1=p_message.substring(0,p_message.indexOf("-")).trim();
            while(expr2.indexOf("-")!=-1){
                postIdx=expr2.indexOf("-");
                if(postIdx ==0){
                    negPostNum = true;
                    expr2=expr2.substring(1,expr2.length()).trim();
                    continue;
                }
                if(postIdx == -1){
                    negPostNum = true;
                    expr2=expr2.substring(1,expr2.length()).trim(); 
                } else{
                    expr2=expr2.substring(0,postIdx).trim();
                }
            }
            if(expr1.indexOf("-")!=-1 || expr1.indexOf("+")!=-1){
                subIdx=expr2.lastIndexOf("-");
                addIdx =expr2.lastIndexOf("+");
                preIdx=Math.max(addIdx, subIdx);
                expr1=expr1.substring(preIdx+1,expr1.length()).trim();
            }

            if(expr1 !=null && expr1.length()>0 && expr2 !=null && expr2.length()>0){
                if(negPostNum){
                    splitMsg=expr1+"--"+expr2;
                    subVal = Double.parseDouble(expr1) - Double.parseDouble("-"+expr2);
                } else{
                    splitMsg=expr1+"-"+expr2;
                    subVal = Double.parseDouble(expr1) - Double.parseDouble(expr2);
                }
                return p_message = p_message.replace(splitMsg,String.valueOf(subVal));

            } else {
                return p_message+"Terminate";
            }
        }
        //----------------------------------------------------------------------------------------
        //This method returns the value of division between two expressions

        public String getDiv(String p_message) throws Exception
        {
            splitMsg=   expr1=expr2         = null;
            subIdx=addIdx=postIdx=preIdx    = 0;
            int multiPres,addPres,subPres   = 0;
            int postIdxChk,preIdxChk        = 0;
            double divVal                   = 0.0;

            expr2=p_message.substring(p_message.indexOf("/")+1,p_message.length()).trim();
            expr1=p_message.substring(0,p_message.indexOf("/")).trim();

            if(expr2.indexOf("*")!=-1 || expr2.indexOf("+")!=-1 || expr2.indexOf("-")!=-1){

                multiPres=expr2.indexOf("*");
                addPres=expr2.indexOf("+");
                subPres=expr2.indexOf("-");
                postIdxChk=Math.min(multiPres==-1?99999999:multiPres, addPres==-1?99999999:addPres);
                postIdx = Math.min(postIdxChk, subPres==-1?99999999:subPres);
                expr2=expr2.substring(0,postIdx).trim();

            }
            if(expr1.indexOf("*")!=-1|| expr1.indexOf("+")!=-1 || expr1.indexOf("-")!=-1){
                    multiPres=expr1.lastIndexOf("*");
                    addPres=expr1.lastIndexOf("+");
                    subPres=expr1.lastIndexOf("-");
                    preIdxChk=Math.max(multiPres, addPres);
                    preIdx = Math.max(preIdxChk, subPres);
                    expr1=expr1.substring(preIdx+1,expr1.length()).trim();

            }
            splitMsg=expr1+"/"+expr2;
            divVal = Double.parseDouble(expr1) / Double.parseDouble(expr2);
            return p_message = p_message.replace(splitMsg,String.valueOf(divVal));
        }
        //----------------------------------------------------------------------------------------
        //This method returns the value of multiplication between two expressions
        public String getMult(String p_message) throws Exception
        {
            splitMsg=expr1=expr2         = null;
            subIdx=addIdx=postIdx=preIdx = 0;
            int addPres,subPres,multiPres= 0;
            int postIdxChk,preIdxChk     = 0;
            double multiVal              = 0.0;

            expr2=p_message.substring(p_message.indexOf("*")+1,p_message.length()).trim();
            expr1=p_message.substring(0,p_message.indexOf("*")).trim();
            if(expr2.indexOf("+") !=-1 || expr2.indexOf("-")!=-1 || expr2.indexOf("*")!=-1){

                    addPres=expr2.indexOf("+");
                    subPres=expr2.indexOf("-");
                    multiPres=expr2.indexOf("*");
                    postIdxChk=Math.min(multiPres==-1?99999999:multiPres, addPres==-1?99999999:addPres);
                    postIdx=Math.min(subPres==-1?99999999:subPres, postIdxChk);
                    expr2=expr2.substring(0,postIdx).trim();

            }
            if(expr1.indexOf("+")!=-1 || expr1.indexOf("-")!=-1){

                    addPres=expr1.lastIndexOf("+");
                    subPres=expr1.lastIndexOf("-");
                    multiPres=expr2.indexOf("*");
                    preIdxChk=Math.max(multiPres, addPres);
                    preIdx = Math.max(preIdxChk, subPres);
                    expr1=expr1.substring(preIdx+1,expr1.length()).trim();

            }
            splitMsg=expr1+"*"+expr2;
            multiVal = Double.parseDouble(expr1) * Double.parseDouble(expr2);
            return p_message = p_message.replace(splitMsg,String.valueOf(multiVal));

        }

        //----------------------------------------------------------------------------------------
        //This method writes the result of calculation to response queue
        public void writeResponse(String p_response,String p_l_correlationID) throws Exception{

        QueueConnectionFactory  queueConnectionFactory = null;
        QueueConnection         queueConnection = null;
        QueueSession            queueSession    = null;
        Queue                   queue           = null;
        QueueSender             queueSender     = null;
        TextMessage             message         = null;
        String                  l_txtMessage        = "";
        String                  l_corr_id       = "";

        try {

            jndiContext = new InitialContext(properties);
            queueConnectionFactory = (QueueConnectionFactory)
                    jndiContext.lookup("jms/connFactory");//Mention actual connection factory name
            queue = (Queue) jndiContext.lookup(new String("jms/respQueue")); //Mention actual connection queue name
        } catch (NamingException e) {
             e.printStackTrace();
             throw e;
        }

        l_corr_id   = p_l_correlationID;
        l_txtMessage= p_response;   
        try {
                queueConnection = queueConnectionFactory.createQueueConnection();
                queueSession    = queueConnection.createQueueSession(false,Session.AUTO_ACKNOWLEDGE);
                queueConnection.start();
                queueSender     = queueSession.createSender(queue);
                message         = queueSession.createTextMessage();
                message.setJMSCorrelationID (l_corr_id != null?l_corr_id:"");
                message.setText(l_txtMessage);
                queueSender.send(message);

            } catch (JMSException e) {
                e.printStackTrace ();
                throw e;
            } finally {
                if (queueConnection != null) {
                    try {
                        queueConnection.close();
                    } catch (JMSException e) {}
                }
                if (queueSession != null) {
                    try {
                        queueSession.close();
                    } catch (JMSException e) {}
                }
                if (queueSender != null) {
                    try {
                        queueSender.close();
                    } catch (JMSException e) {}
                }

            }
    }
    //----------------------------------------------------------------------------------------      
    public void ejbRemove () 
            throws EJBException {

            }
    //----------------------------------------------------------------------------------------  
    public void ejbCreate () 
            throws EJBException {


            }
    //----------------------------------------------------------------------------------------  
    public void setMessageDrivenContext (
            MessageDrivenContext    p_context
        ) throws EJBException {


        }
    //----------------------------------------------------------------------------------------
}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Tiny thing, but probably you should use whitespaces consistently: in some places, you have unnecessary indentations, and missing spaces (before and after "=" and ","). \$\endgroup\$
    – Olli
    Jan 30, 2014 at 18:05

1 Answer 1

3
\$\begingroup\$

Some notes:

  1. The get* methods do not look too easy to understand. I bet that there is some library which could do the calculations for you (here a question about it on Stack Overflow), so using one would improve both readability and maintainability of your code (since you need less code) and save you a few days too.

    See also: Effective Java, 2nd edition, Item 47: Know and use the libraries (The author mentions only the JDK's built-in libraries but I think the reasoning could be true for other libraries too.)

  2. From Code Complete, 2nd Edition, p761:

    Use only one data declaration per line

    [...] It’s easier to modify declarations because each declaration is self-contained.

    [...]

    It’s easier to find specific variables because you can scan a single column rather than reading each line. It’s easier to find and fix syntax errors because the line number the compiler gives you has only one declaration on it.

  3.  } catch (final EJBException e) {
         e.printStackTrace();
         throw new EJBException();
     } ...
    

    Logging and rethrowing an exception usally leads to logging it twice (application servers logs system output too) which means harder maintenance (two stacktrace for every error). Furthermore, if you rethrow an exception set its cause:

    } catch (final EJBException e) {
        throw new EJBException(e);
    } ...
    

    It helps debugging a lot.

    Other references:

  4. l_message is unnecessary, you could use p_message instead of it.

  5. The declaration of l_result could be inside the try block. Try to minimize the scope of local variables. It's not necessary to declare them at the beginning of the method, declare them where they are first used. (Effective Java, Second Edition, Item 45: Minimize the scope of local variables)

    final String result = calculate(requestStr);
    writeResponse(result, correlationID);
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.