I have four of this really long if-else code. How can I refactor this code?
if (objectcameraangle > 140)
{
if (Vector3.Distance(joint1, joint2) > 0.5)
{
Rfootikcontroller.transform.Translate((scx / 10000), scy / 10000, 0);
}
else
{
Rfootikcontroller.transform.Translate((scx / 800), scy / 800, 0);
}
}
else if (objectcameraangle < 35)
{
if (Vector3.Distance(joint1, joint2) > 0.5)
{
Rfootikcontroller.transform.Translate((-scx / 10000), scy / 10000, 0);
}
else
{
Rfootikcontroller.transform.Translate((-scx / 800), scy / 800, 0);
}
}
else if (objectcameraangle > 35 && objectcameraangle < 140 && signed > 0)
{
if (Vector3.Distance(joint1, joint2) > 0.5)
{
Rfootikcontroller.transform.Translate(0, scy / 10000, (-scx / 10000));
}
else
{
Rfootikcontroller.transform.Translate(0, scy / 800, (-scx / 800));
}
}
How I can rewrite it to something more nice . Trying to learn here something new . Thanks for help.
CodePudding user response：
Something like this?
var distance = Vector3.Distance(joint1, joint2);
var divisor = distance > 0.5 ? 10000 : 800;
var scx2 = scx / divisor;
var scy2 = scy / divisor;
if (objectcameraangle < 35)
{
Rfootikcontroller.transform.Translate(-scx2, scy2, 0);
}
else if (objectcameraangle < 140)
{
if (signed > 0)
Rfootikcontroller.transform.Translate(0, scy2, -scx2);
}
else
{
Rfootikcontroller.transform.Translate(scx2, scy2, 0);
}
CodePudding user response：
If you're using a recent version of C# (8.0 I think?), you can try something like this:
var denominator = (Vector3.Distance(joint1, joint2) > 0.5 ? 10000 : 800);
switch (true) {
case var _ when objectcameraangle > 140:
Rfootikcontroller.transform.Translate((scx / denominator), scy / denominator, 0);
break;
case var _ when objectcameraangle < 35:
Rfootikcontroller.transform.Translate((-scx / denominator), scy / denominator, 0);
break;
case var _ when objectcameraangle > 35 && objectcameraangle < 140 && signed > 0:
Rfootikcontroller.transform.Translate(0, scy / denominator, (-scx / denominator));
break;
}
Hopefully that compiles... I don't have all of your objects available, obviously, so I couldn't check myself.
CodePudding user response：
Primarily, I would like to say that i agree @Victor answer, because that code is very easy to understand than the original code. However @wildbeast is trying to learn for something new, this way, I'll propose an Object Oriented way to solve this problem. Before i start, I'll alert that the code i'll propose below maybe is not the right solution to the exact problem showed by @wildbeast, its only a method that i commonly have seen to solve problems with multiples "if-else" in a Oriented-object way.
First, I'll identify and isolate the duplicate rules of original code and store then in variables (like @Victor alread did). Later I'll identify each part of this code that can be an object isolated with its own rules. For each business object i'll implement an interface that i'll be injected by some DI container. Finally I'll create an Resolver class that will take the necessary parameters to discovery the right choise to use.
Bellow, the proposed solution:
class Program
{
static void Main(string[] args)
{
//...Initialize variables joint1, joint2, etc...
//get instaces from DI container
var kernel = new StandardKernel();
kernel.Load(Assembly.GetExecutingAssembly());
var resolver = kernel.Get<IObjectCamAngleResolver>();
var signed = 1;
var distance = Vector3.Distance(joint1, joint2);
var divisor = distance > 0.5 ? 10000 : 800;
var scx2 = scx / divisor;
var scy2 = scy / divisor;
var entrance = new EntranceDto
{
Scx = scx2,
Scy = scx2,
Signed = signed
};
//Call resolver to return the correct implementation;
var objectCamAngle = resolver.Resolve(130);
var values = objectCamAngle?.GetValuesToTranslateRFootikController(entrance);
if (values != null)
{
Rfootikcontroller.transform.Translate(values.ValueA, values.ValueB, values.Valuec);
}
}
}
Resolver and Isolated rules:
public class ObjectCamAngleResolver : IObjectCamAngleResolver
{
private readonly IEnumerable<IObjectCamAngle> objectCamAngleAvailables;
public ObjectCamAngleResolver(IEnumerable<IObjectCamAngle> objectCamAngleAvailables)
{
this.objectCamAngleAvailables = objectCamAngleAvailables;
}
public IObjectCamAngle Resolve(decimal value)
{
return objectCamAngleAvailables?.Where(x => x.Accept(value))?.FirstOrDefault();
}
}
public interface IObjectCamAngleResolver
{
IObjectCamAngle Resolve(decimal value);
}
public class ObjectCamAngleRange140OrHigher : IObjectCamAngle
{
public bool Accept(decimal value)
{
return value > 140;
}
public ScaleValuesDto GetValuesToTranslateRFootikController(EntranceDto entrance)
{
return new ScaleValuesDto
{
ValueA = entrance.Scx,
ValueB = entrance.Scy,
Valuec = 0
};
}
}
public class ObjectCamAngleRange140OrSmaller : IObjectCamAngle
{
public bool Accept(decimal value)
{
return value < 140 && value > 35;
}
public ScaleValuesDto GetValuesToTranslateRFootikController(EntranceDto entrance)
{
return entrance.Signed > 0
? new ScaleValuesDto
{
ValueA = 0,
ValueB = entrance.Scy,
Valuec = -entrance.Scx
}
: null;
}
}
public class ObjectCamAngleRange35OrSmaller : IObjectCamAngle
{
public bool Accept(decimal value)
{
return value < 35;
}
public ScaleValuesDto GetValuesToTranslateRFootikController(EntranceDto entrance)
{
return new ScaleValuesDto
{
ValueA = -entrance.Scx,
ValueB = entrance.Scy,
Valuec = 0
};
}
}
public interface IObjectCamAngle
{
ScaleValuesDto GetValuesToTranslateRFootikController(EntranceDto entrance);
bool Accept(decimal value);
}
public class EntranceDto
{
public decimal? Signed { get; set; }
public decimal Scx { get; set; }
public decimal Scy { get; set; }
}
public class ScaleValuesDto
{
public decimal ValueA { get; set; }
public decimal ValueB { get; set; }
public decimal Valuec { get; set; }
}
Ninject container configuration:
public class Bindings : NinjectModule
{
public override void Load()
{
Bind<IObjectCamAngleResolver>().To<ObjectCamAngleResolver>();
Bind<IObjectCamAngle>().To<ObjectCamAngleRange140OrHigher>();
Bind<IObjectCamAngle>().To<ObjectCamAngleRange140OrSmaller>();
Bind<IObjectCamAngle>().To<ObjectCamAngleRange35OrSmaller>();
}
}
To install ninject:
Install-Package Ninject -Version 3.3.5
CodePudding user response：
Looking at your code I see two things:
Almost all of that code is for all intents and purposes, simply assigning 4 different variables, and
There are fall-through/no-op conditions: When
scxis exactly35or140, orsignedis non-positive.
I would isolate the assignment of each variable and invoke the Translate() method at the end, thus:
int angle = objectcameraangle // because, too many letters (and runtogetherwords)
int d = Vector3Distance > 0.5
? 10000
: 800
;
int? x = angle > 140 ? scx / d
: angle < 35 ? - scx / d
: angle > 35 && angle < 140 && signed > 0 ? 0
: null // TODO what happens when objectcameraangle is exactly 35 or 140, or is signed <= 0?
;
int? y = angle > 140 ? scy / d
: angle < 35 ? scy / d
: angle > 35 && angle < 140 && signed > 0 ? scy / d
: null // TODO what happens when objectcameraangle is exactly 35 or 140, or is signed <= 0?
;
int? z = angle > 140 ? 0
: angle < 35 ? 0
: angle > 35 && angle < 140 && signed > 0 ? - scx / d
: null // TODO what happens when objectcameraangle is exactly 35 or 140, or is signed <= 0?
;
// Wrapped in a guard clause to protect against the fall-through condition(s) noted above
if ( x != null && x != null && z != null ) {
Rfootikcontroller.transform.Translate( x, y, z );
}
You could then extract the logic for each of the 4 variable assignments out to individual methods, making them simple and easily testable. That would give you something like this:
static int SelectDistance( double v3Distance )
{
return Vector3Distance > 0.5
? 10000
: 800
;
}
static int SelectValue( angle int , signed int, v1 int, v2 int , v3 int , v4 int)
{
return angle > 140 ? v1
: angle < 35 ? v2
: angle > 35 && angle < 140 && signe > 0 ? v3
: v4
}
.
.
.
int angle = objectcameraangle // because, too many letters (and runtogetherwords)
int d = SelectDistance( Vector3Distance );
int? x = SelectValue( angle, signed, scx/d, -scx/d, 0, null );
int? y = SelectValue( angle, signed, scy/d, scy/d, scy/d, null );
int? z = SelectValue( angle, signed, 0, 0, -scx/d, null );
// Wrapped in a guard clause to protect against the fall-through condition(s) noted above
if ( x != null && x != null && z != null ) {
Rfootikcontroller.transform.Translate( x, y, z );
}