• Post Reply Bookmark Topic Watch Topic
  • New Topic

Is it correct to break method into two and move the second to helper class?  RSS feed

 
Monica Shiralkar
Ranch Hand
Posts: 922
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a class which has big methods. I have broken the big methods into the two methods in the same class and tested it. It worked fine and also the code looks cleaner. After this I moved the second method into helper class. Is it a correct way of coding?

Orignal code was like this:


I changed it to below:



I moved the method meth to another class and named it MyClassHelper.


}



It works fine . Is it a correct way of coding?

thanks
 
Nguyen Tuyen
Ranch Hand
Posts: 30
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
When you have some line of codes which do solve a common problem, you can use it more and more times then you should split it into a Helper class like you did. But remember that this method must be a common method for all class can use, not in case inheritance. For example, you have a supper class A, with method called run() and all child of it also using this method, then you must put it there not moving it into a helper class.
Another comment to your code, when you have split code to the helper function, remember that this function only do a very common function, it will not process any particular problems, you must make it as general as you can. All particular problems (if need for run helper) must pre-process then pass into helper function via it's arguments.
As I see in your code, the function 'doSomeThing()' after you split the helper, this function does not have any code you just call the helper, it will be not a good practice.
Your code should be like this:
 
Monica Shiralkar
Ranch Hand
Posts: 922
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks. As per your reply it means the functionality in helper class methods should be fir e.g reusable methods.  Also I read that helper classes methods  should not have any relation tho business logic and class should not need to be instantiated by creating objects.    In my helper class I have methods which needs be called in the static way so it looks fine with thus aspect. But the methods are not exactly reusable methods. That may be used just once in code. The only purpose I am achieving by this is that code is that the main class looks a lot cleaner where bunch of work is done by helper class methods and methods of main class just calls these static methods which does major work. So the code looks lot cleaner. Should I remand my helper class to something other than with name helper because I do not gave reusable methods and just using it for cleaner  main class?
 
Campbell Ritchie
Marshal
Posts: 56525
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Nguyen Tuyen wrote:. . . a supper class A, with method called run()
Interesting spelling; I think you mean superclass.
and all child of it  . . .
Although many people say parent‑child, those are maybe not the best terms to use. A subclass is a specialised kind of superclass, and you cannot usually say that a child is a specialised type of their parent(s). The Java® terminology is superclass‑subclass, but maybe what the C# people say is better: base class‑derived class.
 
Campbell Ritchie
Marshal
Posts: 56525
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Why do people say main class? Yes, I know NetBeans creates a class called Main, but that isn't what you are talking about. That Main class (capital M) shou‍ld contain a main method and nothing else. If you only have one class, you might call that a God class, which is a well‑known anti‑pattern.

As a rule of thumb, consider whether a method is a function. That means all its input information comes from its parameters (or constants) and all the information it produces goes into its return value. In which case it is a 1368 in the most dubious classification of methods ever seen. In which case you can consider making it static. If you now have static methods which are used in more than one place, yes, consider creating a utility class to encapsulate them.
 
Monica Shiralkar
Ranch Hand
Posts: 922
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If you now have static methods which are used in more than one place, yes, consider creating a utility class to encapsulate them.


Rest of the things is correct. But the static methods which I have are used only once. In old code there was a class with big methods and code was not looking clean. I moved most of the code to another helper class and called the methods of helper class from the class. Now the code looks more cleaner. Is it a correct way?
thanks
 
Campbell Ritchie
Marshal
Posts: 56525
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
As a general rule of thumb, if the code looks cleaner it is better
 
Randall Twede
Ranch Hand
Posts: 4696
8
Java Scala
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
some general rules about this.
a class should do one thing.
even more so a method should do one thing.
try to keep your classes and methods small and self-contained.
 
Monica Shiralkar
Ranch Hand
Posts: 922
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie says
As a general rule of thumb, if the code looks cleaner it is better


Thanks. So can the name of Helper class still be some Helper only in this case. (when the static methods are actually being used only once in code)

Randall Twede says
some general rules about this.
a class should do one thing.
even more so a method should do one thing.
try to keep your classes and methods small and self-contained.


Thanks.

In case a class should do only one kind of thing should there be multiple helpers in this case doing one kind of thing each?
 
Dave Tolls
Ranch Foreman
Posts: 3056
37
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Personally, break the code into nice simple methods (essentially the description Randall suggests).
If this is object code (ie non-static) then any sub methods created as part of this breakdown will be non-static except in rare cases where you identify common helper code (code likey to be usable elsewhere) which would then be removed and placed in a proper utility class.  I would not (in general) make methods static in my model/processing classes simply because they could be.
 
Campbell Ritchie
Marshal
Posts: 56525
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I think maybe, consider that an object represents one thing. Utility classes don't represent anything at all; which is why you don't usually instantiate them. Utility classes exist to collect methods and constants for other code to use, so there is nothing wrong with collecting multiple methods in a utility class. Look at classes like java.lang.Math, java.util.Collections and java.util.Arrays for examples. Each contains multiple methods which do different things from one another, but they all have the common feature that they are used on the same sort of target.
 
Stephan van Hulst
Saloon Keeper
Posts: 7969
143
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Campbell Ritchie wrote:Utility classes don't represent anything at all; which is why you don't usually instantiate them.

I'd go one step further and say that you should make it impossible to instantiate them, by giving it a single private constructor that does nothing.
 
Monica Shiralkar
Ranch Hand
Posts: 922
2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Stephan van Hulst wrote:
Campbell Ritchie wrote:Utility classes don't represent anything at all; which is why you don't usually instantiate them.

I'd go one step further and say that you should make it impossible to instantiate them, by giving it a single private constructor that does nothing.


Thanks. What issue can occur if we do not keep a private constructor..
 
Campbell Ritchie
Marshal
Posts: 56525
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Inappropriate use of object references to call static methods and reference static fields:-Unnecessary creation of objects:-Unnecessary access to methods:-I don't think any of those will be disastrous, but they are all bad style.
For the same reason, you never override any Object methods in utility classes. If you cannot create any instances, you can never call equals(), nor hashCode(), nor toString(), so overloading them is pointless.

Note it would have been possible to design the Math class as an ordinary concrete class:-But that isn't how it was designed. I suspect it is because in C/C++ mathematical functions are implemented as “naked” functions in the math.h header file and the easiest way to implement such multiple functions is as a utility class. It certainly makes it easier to implement functions like max and min.
 
Campbell Ritchie
Marshal
Posts: 56525
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
A few minutes ago, I wrote:. . . Unnecessary access to methods:-. . .
. . . and forgot to explain that properly. That is of course indirect access to the toString method. There is no need for the Math class to print its details, because it hasn't got any program state of its own.
 
With a little knowledge, a cast iron skillet is non-stick and lasts a lifetime.
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!