is it candidate for being static

Hi,
  I have a fn like this

public boolean fnA()
{

   new CommonUtil(props).validateReq(tn,tracknum,state);

}
the validateReq in CommonUtil class validates request to see if tn,tracknum and state are in valid format.Should the validateReq be defines as static? This method is called only once and not referenced anywhere else
skn73Asked:
Who is Participating?
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

objectsCommented:
if it doesn't require access to any of CommonUtil's member vars then yes it can be static.
If it does need to use member vars then it cannot.
0
skn73Author Commented:
by it you mean fnA()?
0
objectsCommented:
no the method you want to be static, validateReq()
0
Get expert help—faster!

Need expert help—fast? Use the Help Bell for personalized assistance getting answers to your important questions.

zzynxSoftware engineerCommented:
>> validateReq ... validates request to see if tn,tracknum and state are in valid format.
>> Should the validateReq be defines as static?
if validateReq just needs tn, tracknum and state and can do what it has to on base of just that info,
yes, it is a serious candidate to be defined as static. That way you can call it like
       CommonUtil.validateReq(tn, tracknum, state);
and don't need a CommonUtil object to call it on.
0
objectsCommented:
Easiest way to check if it can be declared static is to simply declare it static and compile it.
It won't compile if it uses any instance vars or methods.
0
krakatoaCommented:
It's not whether it (validateReq(...)) uses the instance members of *CommonUtil* or not, it's about whether it uses the vars from the *enclosing class of fnA() or not*; like zzynx says you don't need the object, you just call CommonUtil.validateReq(...).

You are maybe thinking of the case where validateReq(...) could be in the client class itself, in which case you would only make it static if the objects of that class all shared the same instance var values, otherwise you need to make the method an instance method so that it handled the current values for those instance vars using their correct values.
0
objectsCommented:
> it's about whether it uses the vars from the *enclosing class of fnA() or not

fnA() has nothing to do with it.
0
zzynxSoftware engineerCommented:
>> It's not whether it (validateReq(...)) uses the instance members of *CommonUtil* or not
It's - like objects said - whether CommonUtil member vars are used or not. Unless they are static too.
0
objectsCommented:
> whether CommonUtil member vars are used or not. Unless they are static too.

If they are static then they are not member vars.
And its not only member vars, but also member methods.
0
krakatoaCommented:
That's not what I am saying.

IMHO the questioner may have a bit of confusion in his mind - CommonUtil may as well have all its members as static, since as zzynx already said, you don't need an instance of it.

What I am saying is that the question that is being asked only arises when the values of the instance vars are different, in which case they can not be static. But the reason it doesnt apply to CommonUtil, is that CommonUtil is common, and it is simply a class that exists as a utility that other classes can call on.

>> fnA() has nothing to do with it.

As for this, I never said that fnA() had anything to do with it : I said the **enclosing class** of fnnA().

0
objectsCommented:
> CommonUtil may as well have all its members as static, since as zzynx already said, you don't need an instance of it.

there is nothing posted to suggest that.

> As for this, I never said that fnA() had anything to do with it : I said the **enclosing class** of fnnA().

sorry, the **enclosing class** of fnnA() has nothing to do with it.
0
krakatoaCommented:
There doesn't need to be anything posted to suggest it - the reason for having a CommonUtil is so that it is inclusive (common / static), and not exclusive (instance / non-static). ;)

>> sorry, the **enclosing class** of fnnA() has nothing to do with it.

The enclosing class of fnA() has a lot to do with it. That is where the values to be checked are coming from.

Having said all that, this is a peculiar question, since if "tn,tracknum,state" are instance vars, then the only place that really knows whether they are correct or not would normally be the instance itself.
0
zzynxSoftware engineerCommented:
skn73, can you say something? ;°)
0
objectsCommented:
> the reason for having a CommonUtil is so that it is inclusive (common / static), and not exclusive (instance / non-static).

Again there is nothing to suggest that, in fact the only mention shown for CommonZUtils suggests otherwise.

> The enclosing class of fnA() has a lot to do with it. That is where the values to be checked are coming from.

incorrect

> since if "tn,tracknum,state" are instance vars, then the only place that really knows whether they are correct or not would normally be the instance itself.

again, they have nothing to do with it.

0
krakatoaCommented:
>> again, they have nothing to do with it.
 LOL.
0
krakatoaCommented:
We could use a good gc() here at the moment - skn73 - you there?
0
OrtokoboldCommented:
IMHO it's more design question, than language-syntax question. If "validateReq" is to used only once, why to put it to the "CommonUtils" class? Maybe it just should be a private member of the class where it is used (the most obvious solution).
0
objectsCommented:
The number of times it is used is not the issue here, it is whether it can be made static or not which is dictated by whether it makes any use of member vars or members. And as we don't have that information it can be determined either way.
As I said earlier an easy test is to make it static and attempt to compile it.
0
krakatoaCommented:
It's not really to do wtih "language syntax". It could be regarded as a design issue in a way - if by design you accept that this question is predetermined by OOP principles themselves anyway.
0
krakatoaCommented:
It's true that *mechanically* the class will not compile properly if it makes use of instance vars with incompatible modifiers, but that is *still* not the issue - the issue is whether the utility class is using the vars of the enclosing class of fnA().

How can the utility keep track of all the instance vars from all  the objects it is supposed to check, *unless* all the checking does is to validate them against a range - or format, as the question in fact states. If it is a question of checking the *format* then that's fine, because presumably, that is at least fixed.
0
zzynxSoftware engineerCommented:
Hey guys, don't you think you better stop posting?
0
objectsCommented:
> the issue is whether the utility class is using the vars of the enclosing class of fnA().

sorry but thats incorrect. the utility class has no access or knowledge of the calling classes vars.

0
krakatoaCommented:
>> sorry but thats incorrect. the utility class has no access or knowledge of the calling classes vars

Where are you saying that this function call :

>> .validateReq(tn,tracknum,state);

obtains its params from?
0
krakatoaCommented:
>> Hey guys, don't you think you better stop posting?

Not sure - what do you think? ;)
0
krakatoaCommented:
The real point is it doesn't matter whether the utility is static or not, since you call into it from outside anyway. Your concerns about "static" are only relevant if you were talking about a utility method *inside your current object class* - in which case this could very well be made static, if all the method does is to check the values against a range or a format principle.
0
skn73Author Commented:
Sorry! I am seeing this postings only now ! Thanks for all ur input ..
public  boolean fnA( String tn, String state, String txnum) throws ServicesApplicationException
{
  try {        
 new CommonUtil(props).validateReq(ServicesConstants.DATA_NOT_APPLICABLE,tn,state);
}
catch( ...)

}
0
skn73Author Commented:
common Util.java has
public class CommonUtil {
      
      private Pattern reTn;
      private Pattern reQualTrackNum;
      private Pattern reState;
      private boolean reInit;
      private Props props;
      
      public CommonUtil(Props p)
      {
            props = p;
      }
      
      private synchronized void setupRePatterns() throws ServicesApplicationException
      {
         try
         {
               TraceLog.write(TraceLog.DEBUG, "Initializing regular patterns...");
      
               if (reInit)
                  return;
               reTn = new Perl5Compiler().compile(props.getValue(ServicesConstants.REGEXP_TN_10DIGIT), Perl5Compiler.READ_ONLY_MASK);
               reQualTrackNum = new Perl5Compiler().compile(props.getValue(ServicesConstants.REGEXP_QUAL_TRACK_NO), Perl5Compiler.READ_ONLY_MASK);
               reState = new Perl5Compiler().compile(props.getValue(ServicesConstants.REGEXP_STATE_2CHAR), Perl5Compiler.READ_ONLY_MASK);

               reInit = true;
               return;
         }
        
         catch(Throwable t)
         {
               ServicesError.throwProcessingException(t, "ServiceRequestBean::setupRePatterns()", props);
         }

         return;
      }
      
      public void validateReq(String qualTrackNo, String tn, String state) throws ServicesApplicationException
      {
            String fctStr = "ServiceRequestBean::checkReq()";
      
            PatternMatcher re = new Perl5Matcher();

            if (!reInit)
                  setupRePatterns();
      
            try
            {
               // EDIT THE TELEPHONE NUMBER
               if ( (tn == null) ||
                        (!re.matches(tn, reTn)) )
                        ServicesError.throwServicesException(props.getValue(ServicesConstants.INVALID_DATA_ERROR_CODE),
                                                                         props.getValue(ServicesConstants.INVALID_DATA_ERROR_MSG) + " -- Invalid TN",
                                                                         "The request data is invalid -- TN", fctStr);

               // EDIT THE QUALIFICATION TRACKING NUMBER
              if ( (( qualTrackNo == null) ||  (!re.matches(qualTrackNo, reQualTrackNum)) ) &&
                        (!qualTrackNo.equals(ServicesConstants.DATA_NOT_APPLICABLE)))
                        ServicesError.throwServicesException(props.getValue(ServicesConstants.INVALID_DATA_ERROR_CODE),
                                                                               props.getValue(ServicesConstants.INVALID_DATA_ERROR_MSG) + " -- Invalid Qualification Tracking Number",
                                                                               "The request data is invalid -- Qualification Tracking Number", fctStr);
      

                  // EDIT THE STATE
                  if ( state == null ||
                        (!re.matches(state, reState)) )
                        ServicesError.throwServicesException(props.getValue(ServicesConstants.INVALID_DATA_ERROR_CODE),
                                                                               props.getValue(ServicesConstants.INVALID_DATA_ERROR_MSG) + " -- Invalid State",
                                                                               "The request data is invalid -- State", fctStr);
            }
            catch(ServicesApplicationException sae)
            {
               throw sae;
            }
            catch(Throwable t)
            {
                  ServicesError.throwProcessingException(t, "ServiceRequestBean::checkReq()", props);
            }
            return;
      }
0
zzynxSoftware engineerCommented:
Well, what does it give if you make it static? Does it compile?
0
zzynxSoftware engineerCommented:
No, it won't.
Since in validateReq() you call setupRePatterns(); which is not static.
0
zzynxSoftware engineerCommented:
Main conclusion: you can't (easily) make it static, so better leave it as it is.
0
skn73Author Commented:
can I make commonUtil a singleton
0
skn73Author Commented:
the big problem is many dont like this statement "new CommonUtil(props).validateReq(ServicesConstants.DATA_NOT_APPLICABLE,tn,state);"   starting with new and I am wondering how to make it better?

public  boolean fnA( String tn, String state, String txnum) throws ServicesApplicationException
{
  try {        
 new CommonUtil(props).validateReq(ServicesConstants.DATA_NOT_APPLICABLE,tn,state);
}
catch( ...)

}
0
objectsCommented:
To make it static you'd need to change it to something like:

public class CommonUtil {
     
     private static Pattern reTn;
     private static Pattern reQualTrackNum;
     private static Pattern reState;
     
     private static void setupRePatterns(Properties props) throws ServicesApplicationException
     {
        try
        {
             TraceLog.write(TraceLog.DEBUG, "Initializing regular patterns...");
     
             reTn = new Perl5Compiler().compile(props.getValue(ServicesConstants.REGEXP_TN_10DIGIT), Perl5Compiler.READ_ONLY_MASK);
             reQualTrackNum = new Perl5Compiler().compile(props.getValue(ServicesConstants.REGEXP_QUAL_TRACK_NO), Perl5Compiler.READ_ONLY_MASK);
             reState = new Perl5Compiler().compile(props.getValue(ServicesConstants.REGEXP_STATE_2CHAR), Perl5Compiler.READ_ONLY_MASK);

             reInit = true;
             return;
        }
       
        catch(Throwable t)
        {
             ServicesError.throwProcessingException(t, "ServiceRequestBean::setupRePatterns()", props);
        }

        return;
     }
     
     public static void validateReq(Properties props, String qualTrackNo, String tn, String state) throws ServicesApplicationException
     {
          String fctStr = "ServiceRequestBean::checkReq()";
     
          PatternMatcher re = new Perl5Matcher();

         setupRePatterns(props);
     
          try
          {
             // EDIT THE TELEPHONE NUMBER
             if ( (tn == null) ||
                    (!re.matches(tn, reTn)) )
                    ServicesError.throwServicesException(props.getValue(ServicesConstants.INVALID_DATA_ERROR_CODE),
                                                             props.getValue(ServicesConstants.INVALID_DATA_ERROR_MSG) + " -- Invalid TN",
                                                             "The request data is invalid -- TN", fctStr);

             // EDIT THE QUALIFICATION TRACKING NUMBER
            if ( (( qualTrackNo == null) ||  (!re.matches(qualTrackNo, reQualTrackNum)) ) &&
                    (!qualTrackNo.equals(ServicesConstants.DATA_NOT_APPLICABLE)))
                    ServicesError.throwServicesException(props.getValue(ServicesConstants.INVALID_DATA_ERROR_CODE),
                                                                  props.getValue(ServicesConstants.INVALID_DATA_ERROR_MSG) + " -- Invalid Qualification Tracking Number",
                                                                  "The request data is invalid -- Qualification Tracking Number", fctStr);
     

               // EDIT THE STATE
               if ( state == null ||
                    (!re.matches(state, reState)) )
                    ServicesError.throwServicesException(props.getValue(ServicesConstants.INVALID_DATA_ERROR_CODE),
                                                                  props.getValue(ServicesConstants.INVALID_DATA_ERROR_MSG) + " -- Invalid State",
                                                                  "The request data is invalid -- State", fctStr);
          }
          catch(ServicesApplicationException sae)
          {
             throw sae;
          }
          catch(Throwable t)
          {
               ServicesError.throwProcessingException(t, "ServiceRequestBean::checkReq()", props);
          }
          return;
     }
0
objectsCommented:
> can I make commonUtil a singleton

thats another option, you'd need to add something like the following to your existing code to achieve that:


private static CommonUtils instance = null;

public static CommonUtils getCommonUtils(Properties props)
{
   if (instance==null)
   {
      instance = new CommonUtils(props);
   }
   return instance;
}


then to call it you would use:

CommonUtil.getInstance(props).validateReq(ServicesConstants.DATA_NOT_APPLICABLE,tn,state);
0
OrtokoboldCommented:
So, it IS a design question after all (thanks for telling us! ;-))

Here's my proposition:

1. CommonUtil should be pure helper class. So it should contain only common "static" methods, be delcared as "final", and have private constructor (no instances of this class are allowed).

2. You should create another class for matching you regular expressions. Let's call it "DataValidator". It should have the following methods:

public DataValidator(props); // pass the props to the constructor, set members with regular expressions
boolean isValidQualTrackNum(String qualTrackNum); // return true, if this qual track num is valid
boolean isValidState(String state);
boolean isValidTn(String tn);

// AND:
boolean isValidRequest(String qualTrackNum, state, tn)
{
  return isValidQualTrackNum()
    && isValidState(state)
    && isValidTn(td);
}

3. Now you can call the validateReq like this:

DataValidator dataValidator = new DataValidator(props);

if (!dataValidator.isValidRequest(null, tn, state))
{
  String errorMsg = props.getValue(ServicesConstants.INVALID_DATA_ERROR_MSG)
  String errorCode = props.getValue(ServicesConstants.INVALID_DATA_ERROR_CODE)

  CommonUtils.logError(errorMsg, errorCode);
}

4. Read "Code complete" by Steve McConnell. ;-)
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
skn73Author Commented:
Thanks for all the suggestions guys!! I think I am going to follow Ortokobold and Objects suggestions
0
skn73Author Commented:
and thanks for suggesting that book ! I will give it a try!
0
skn73Author Commented:
so ae you saying the CommonUtil will not have anything to do with Datavalidations except logError?
0
OrtokoboldCommented:
Yes. In my opinion - it shouldn't have anything with data validation.
If you name your class "CommonUtil", it means that this class should hold general-purpose methods, widely used by your other classes. There should be such methods like: "logError", "convertDateToString", "getDBConnection", etc. When you start the new project, you should be able to copy the "CommonUtil" (and similar) classes, and use most of its methods. Will you need the "validateReq" method in your other projects? I don't think so.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
Java

From novice to tech pro — start learning today.

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.