views:

2464

answers:

6

I'm working on a project in C#. The previous programmer didn't know object oriented programming, so most of the code is in huge files (we're talking around 4-5000 lines) spread over tens and sometimes hundreds of methods, but only one class. Refactoring such a project is a huge undertaking, and so I've semi-learned to live with it for now.

Whenever a method is used in one of the code files, the class is instantiated and then the method is called on the object instance.

I'm wondering whether there are any noticeable performance penalties in doing it this way? Should I make all the methods static "for now" and, most importantly, will the application benefit from it in any way?

+1  A: 

It seems silly to me to create an object JUST so you can call a method which seemingly has no side effects on the object (from your description i assume this). It seems to me that a better compromise would be to have several global objects and just use those. That way you can put the variables that normally would be global into the appropriate classes so that they have slightly smaller scope.

From there you can slowly move the scope of these objects to be smaller and smaller until you have a decent OOP design.

Then again, the approach that I would probably use is different ;).

Personally, I would likely focus on structures and the functions which operate on them and try to convert these into classes with members little by little.

As for the performance aspect of the question, static methods should be slightly faster (but not much) since they don't involve constructing, passing and deconstructing an object.

Evan Teran
+6  A: 

From here, a static call is 4 to 5 times faster than constructing an instance every time you call an instance method. However, we're still only talking about tens of nanoseconds per call, so you're unlikely to notice any benefit unless you have really tight loops calling a method millions of times, and you could get the same benefit by constructing a single instance outside that loop and reusing it.

Since you'd have to change every call site to use the newly static method, you're probably better spending your time on gradually refactoring.

stevemegson
+1  A: 

I think you've partially answered this question in the way you asked it: are there any noticeable performance penalties in the code that you have?

If the penalties aren't noticeable, you needn't necessarily do anything at all. (Though it goes without saying the codebase would benefit dramtically from a gradual refactor into a respectable OO model).

I guess what I'm saying is, a performance problem is only a problem when you notice that it's a problem.

sgreeve
+3  A: 

It depends on what else that object contains -- if the "object" is just a bunch of functions then it's probably not the end of the world. But if the object contains a bunch of other objects, then instantiating it is gonna call all their constructors (and destructors, when it's deleted) and you may get memory fragmentation and so on.

That said, it doesn't sound like performance is your biggest problem right now.

Moishe
+3  A: 

I have dealt with a similar problem where i work. The programmer before me created 1 controller class where all the BLL functions were dumped.

We are redesigning the system now and have created many Controller classes depending on what they are supposed to control e.g.

UserController, GeographyController, ShoppingController...

Inside each controller class they have static methods which make calls to cache or the DAL using the singleton pattern.

This has given us 2 main advantages. It is slightly faster(around 2-3 times faster but were talking nanoseconds here ;P). The other is that the code is much cleaner

i.e

ShoppingController.ListPaymentMethods()

instead of

new ShoppingController().ListPaymentMethods()

I think it makes sense to use static methods or classes if the class doesn't maintain any state.

Alex
+1  A: 

Calling an Object method will be slower. There are many factors, You will have to instantiate the class, which require memory allocation in Heap and later deallocation, GC etc. If you have some kind of object caching in backend, use Object it might give you a performance boost. Rule is, if you have to reuse the same object (or object data), or maintain state in multiple calls, use Objects to perform actions.

In case of static method, the pointer will move to Object type, jit the code, execute it and move out. Simple. So rule of thumb is, if you method is not using any object data, just perform an action and return the result, Use static methods. Like Add(int a, int b), all math class methods are static. All CRUD methods can be static implementation.

nin