Refactoring

Unreal Courses Talk
,
#1

HI Ben

I am trying to understand this refactoring process but it seems to not logical…
During this lesson you are in a fact creating more code instead od shoreting it…
Can you explain to me why you create new method?

THANKS

#2

@ben Likes creating functions? Said this before on Udemy but I think he made the code harder to read as the extra function only calls a single function, so you may as well just call that function instead. For comparison here’s my code which does the same thing.

bool ATankPlayerController::GetAimHitLocation(FVector& HitLocation) const
{
	int32 SizeX, SizeY;
	GetViewportSize(SizeX, SizeY);
	FVector2D ScreenLocation(SizeX*CrosshairX, SizeY*CrosshairY);
	FVector WorldLocation, WorldDirection;
	if (DeprojectScreenPositionToWorld(ScreenLocation.X, ScreenLocation.Y, WorldLocation, WorldDirection))
	{
		FHitResult Hit;	
		FVector Start = WorldLocation;
		FVector End = Start + WorldDirection * LineTraceRange;
		if (GetWorld()->LineTraceSingleByChannel(Hit, Start, End, ECC_Visibility))
		{
			HitLocation = Hit.Location;
			return true;
		}
		
	}
	HitLocation = FVector(0);
	return false;
}
#3

I do it to make things self-commenting, but single calls are harder to read I agree. We’ll be re-working this a little soon anyway as we evolve the architecture. Thanks

#4

The call to GetLookDirection is easier to read than the native unreal method. You can forget the code that is in this method once it’s done and only think about the simpler call to it. Like in the native method, you just call in and don’t worry about what is in it. Very useful if we want to call GetLookDirection in several places.
One added benefit of this refactoring is that the useless variable WorldLocation is discarded, meaning we are not wasting memory on it after the method call.
One thing to understand is that the point of refactoring is never to shorten the number of lines.
The point of it is to make quality code, code that reduces risks of bugs, improves readablity, modularity, re-usability, performance optimisation etc.

However I wonder why we are using ScreenLocation and passing it as a parameter. CrosshairX CrosshairY are member variables and GetVIewportSize a method available everywhere in the class and are directly available within the new method. My guess is that maybe we will need that 2dVector after the method call?

1 Like

Privacy & Terms