Too much method extraction imo

regarding the lesson 25, I find too much method extraction, like moving
Camera.main.ScreenPointToRay(Input.mousePosition)
in its own GetMouseRay method

Reusing is fine, but reusing even single line is not a good choice imo, even because it has performance implication (moving the method arguments to the stack and call the method, while we had just to inline the call itself)
(I don’t know how smart is the unity compiler about inlining methods like this)

What do you think?

1 Like

First of all, Welcome back to the forums :slight_smile:
I noticed it had been a while since we saw you!

I did think that refactoring a single line was a bit overkill myself but having worked with Sam before i find he usually has a method to his madness later down the line :slight_smile:
The performance hit is actually quite minimal as you say the unity compiler is actually quite clever in this so we do get away with a lot on that side of things.

Obviously i dont know all the details on how unity does it i just seemingly remember it being mentioned in multiple courses and not just gamedev.tv ones either so it must hold some truth :slight_smile:

Hope you are enjoying the new content and once again welcome back :slight_smile:

Thank you for your reply.
Yeah I started this course long time ago but I struggle with finding the time needed to carry on, now I reinstalled latest version of unity and wanted to refresh my (little) knowledge about Unity because I think I’ll need it soon (I probably will implement a library that will be used to interact a blockchain platform (Stratis) with Unity so why not coming back here)

I’m an expert C# dev and so I felt the urge to comment something about these “issues”, e.g. without having any indeep knowledge of Unity, I started already to cache my GetComponent calls in Start() method, creating Extensions and so on (e.g. the GetMouseRay could be an extension because can be used by many classes)

On the other hand, I understand that it’s hard to handle a course when in every lecture there is some code that gets refactored because the goal is different and I can understand that having a “shortcut” where you prepare your code in a specific way because you know you’ll need it done that way later on is handy, while having to create optimized code for every lecture takes more time.
Anyway could be worthy to say in the course that “we are not writing optimized code at first shot because we are going to change it in next lectures” or something like that (like I outlined in the other post I did here InteractWithCombat mouse check should be done outside of foreach loop )
cheers

1 Like

@sampattuzzi thought to pop you in on this one :slight_smile:

I think the cost here is negligible but you are right that there might be some. I dare say the C# compiler should be able to inline though in this case.

The main reason we do it here is for legibility not reuse. By pulling it into a separate method it instantly becomes more obvious what we want to do.

1 Like

Just wanted to chip in and say that Sam’s teaching this has actually been really beneficial to making my own code more readable, and I’ve been coding for 15 years. I used to only focus on re-use, but now I’m extracting a lot more methods just to improve readability and it really has helped.

There may or may not be some minuscule performance differences (most likely not due to compiler optimizations), but I would never teach someone premature optimization over code legibility. If/when performance comes into question then it is going to be extremely unlikely that inlining code is the solution.

4 Likes

There is no absolute truth, especially on programming, so here we are in the “habit room” and in situations where the reason is just make it clear the intention, then I’m not refactoring a self explicatory code, especially if single lined and I’m not going to reuse it (but in this specific scenario we may even reuse it, this is why I talked about having an Extension method on Component class).

The problem with porting the code as if it was a description of something, can cause trouble when you start working with more complex classes because that way we are adding verbosity to obvious concepts and you can end up with methods that calls lot of simplier methods, that call other simplier methods, etc…

This kind of programming is what has been teached by Martin Fowler or Uncle Bob in “Extract Till You Drop” and as every kind of programming habit this has pro and cons and of course a war against who support and who boooos it so I don’t want to start a war here but just having a talk :smiley:

Without forgetting that commenting code is something to pursue when something isn’t obvious.

but I would never teach someone premature optimization over code legibility

I agree on this, but I’m not talking about hard optimization, anyway consider that games relies on performance, specially on mobile devices (of course this doesn’t matter on simple games)

As a last line I want to say I’m not against having method that explain what they are doing (I do it a lot myself) but there is a need of balance (enough more than enough).
It could be useful during a learning process like following a course tho.

My 2 cents

3 Likes

Hey guys,

This seems to have developed beyond the question that started and has become a discussion so in that respect i have moved this to the Talk topic.
This way it can still be debated and discussed as to clear it from our Q&A we have to mark a solution and that locks the topic after a day.

Hope thats all good with you guys, Still feeling my way around a little since starting Admin the forums in the last couple of weeks

1 Like

All good points!

1 Like

Privacy & Terms