patterncsharpModerate
Overlap detection for laying out elements on a page
Viewed 0 times
layingelementsoverlapfordetectionpageout
Problem
How would you simplify/improve the readability of this code? Because even after my attempt to improve the readability I think it still looks messy.
```
private bool INTOP = false;
private bool INLEFT = false;
private bool INRIGHT = false;
private bool INBOTTOM = false;
private bool BEYOND = false;
private List BEYONDDO = new List();
private List FULLOUT = new List();
private List TOPLEFTDO = new List();
private List TOPDO = new List();
private List TOPRIGHTDO = new List();
private List LEFTDO = new List();
private List CURRENTPAGEDO = new List();
private List RIGHTDO = new List();
private List BOTTOMLEFTDO = new List();
private List BOTTOMDO = new List();
private List BOTTOMRIGHTDO = new List();
private bool ForeachChildIn(DependencyObject myPage, IList childList)
{
var result = false;
foreach (var item in childList)
{
var itemresult = false;
if (item is Visual && myPage is Visual)
{
var myVisual = item as Visual;
//relative Position des myVisual zu VisualElement
Point relativeSartPoint = myVisual.TransformToAncestor((Visual)myPage)
.Transform(new Point(0, 0));
var aw = (double)myVisual.GetValue(FrameworkElement.ActualWidthProperty);
var ah = (double)myVisual.GetValue(FrameworkElement.ActualHeightProperty);
var relativeEndHorizontal = relativeSartPoint.X + aw;
var relativeEndVertikal = relativeSartPoint.Y + ah;
Point relativeEndPoint = new Point(relativeEndHorizontal, relativeEndVertikal);
INTOP = (PageRect.Top = relativeEndPoint.Y);
INLEFT = (PageRect.Left = relativeEndPoint.X);
BEYOND = (PageRect.Top >= relativeEndPoint.Y)
|| (PageRect.Bottom = relativeEndPoint.X)
```
private bool INTOP = false;
private bool INLEFT = false;
private bool INRIGHT = false;
private bool INBOTTOM = false;
private bool BEYOND = false;
private List BEYONDDO = new List();
private List FULLOUT = new List();
private List TOPLEFTDO = new List();
private List TOPDO = new List();
private List TOPRIGHTDO = new List();
private List LEFTDO = new List();
private List CURRENTPAGEDO = new List();
private List RIGHTDO = new List();
private List BOTTOMLEFTDO = new List();
private List BOTTOMDO = new List();
private List BOTTOMRIGHTDO = new List();
private bool ForeachChildIn(DependencyObject myPage, IList childList)
{
var result = false;
foreach (var item in childList)
{
var itemresult = false;
if (item is Visual && myPage is Visual)
{
var myVisual = item as Visual;
//relative Position des myVisual zu VisualElement
Point relativeSartPoint = myVisual.TransformToAncestor((Visual)myPage)
.Transform(new Point(0, 0));
var aw = (double)myVisual.GetValue(FrameworkElement.ActualWidthProperty);
var ah = (double)myVisual.GetValue(FrameworkElement.ActualHeightProperty);
var relativeEndHorizontal = relativeSartPoint.X + aw;
var relativeEndVertikal = relativeSartPoint.Y + ah;
Point relativeEndPoint = new Point(relativeEndHorizontal, relativeEndVertikal);
INTOP = (PageRect.Top = relativeEndPoint.Y);
INLEFT = (PageRect.Left = relativeEndPoint.X);
BEYOND = (PageRect.Top >= relativeEndPoint.Y)
|| (PageRect.Bottom = relativeEndPoint.X)
Solution
Please, pretty please, use braces. They are totally lacking in your code.
This bit of code is so hard to read, that it hurts my eyes!
There are two major issues with your code.
The first one is obviously the one you pointed out, which is the way you are doing your if's.
The second is the amount of lists that you have in your code.
You also have some instance fields that you shouldn't have, like those pesky boolean values. They can be declared inside a method.
I would replace all those lists with a dictionary. This makes the code become much simpler, as you only have to manage one variable instead of managing n variables.
So your code could be something along those lines.
if (!itemresult)
if (ForeachChildIn(item, item.getChilds().ToList()))
result = true;
else if ((Visibility)item.GetValue(FrameworkElement.VisibilityProperty) == Visibility.Visible)
if (item is Control)
((Control)item).Background = Brushes.LimeGreen;
else if (item is Panel)
((Panel)item).Background = Brushes.LimeGreen;This bit of code is so hard to read, that it hurts my eyes!
There are two major issues with your code.
The first one is obviously the one you pointed out, which is the way you are doing your if's.
The second is the amount of lists that you have in your code.
You also have some instance fields that you shouldn't have, like those pesky boolean values. They can be declared inside a method.
I would replace all those lists with a dictionary. This makes the code become much simpler, as you only have to manage one variable instead of managing n variables.
So your code could be something along those lines.
public enum Position{
LEFT, TOP_LEFT, TOP, TOP_RIGHT, RIGHT,
BOTTOM_RIGHT, BOTTOM, BOTTOM_LEFT,
FULL_OUT, BEYOND, IN
}
private Position[] avaliablePositions = new Position[]{
LEFT, TOP_LEFT, TOP, TOP_RIGHT, RIGHT,
BOTTOM_RIGHT, BOTTOM, BOTTOM_LEFT,
FULL_OUT, BEYOND, IN
};
private Dictionary objectsForPosition = new Dictionary>();
private Position GetPositionWithinPageRect(Point startPoint, Point endPoint){
bool inTop = (PageRect.Top = endPoint.Y);
bool inLeft = (PageRect.Left = endPoint.X);
bool beyond = (PageRect.Top >= endPoint.Y)
|| (PageRect.Bottom = endPoint.X)
|| (PageRect.Right childList)
{
var result = false;
//move this on you ctor
for(int i = 0; i ());
}
foreach (var item in childList)
{
var myVisual = item as Visual;
if(myVisual == null) continue;
Point relativeSartPoint = myVisual.TransformToAncestor((Visual)myPage)
.Transform(new Point(0, 0));
var aw = (double)myVisual.GetValue(FrameworkElement.ActualWidthProperty);
var ah = (double)myVisual.GetValue(FrameworkElement.ActualHeightProperty);
Point relativeEndPoint = new Point(relativeSartPoint.X + aw, relativeSartPoint.Y + ah);
Position position = GetPositionWithinPageRect(relativeSartPoint, relativeEndPoint);
if(position != Position.IN){
if (!ForeachChildIn(item, item.getChilds().ToList())){
if ((Visibility)item.GetValue(FrameworkElement.VisibilityProperty) == Visibility.Visible){
if (item is Control)
((Control)item).Background = Brushes.LimeGreen;
else if (item is Panel)
((Panel)item).Background = Brushes.LimeGreen;
}
}
return false;
}else{
objectsForPosition[position].add(item);
return true;
}
}
}Code Snippets
if (!itemresult)
if (ForeachChildIn(item, item.getChilds().ToList()))
result = true;
else if ((Visibility)item.GetValue(FrameworkElement.VisibilityProperty) == Visibility.Visible)
if (item is Control)
((Control)item).Background = Brushes.LimeGreen;
else if (item is Panel)
((Panel)item).Background = Brushes.LimeGreen;public enum Position{
LEFT, TOP_LEFT, TOP, TOP_RIGHT, RIGHT,
BOTTOM_RIGHT, BOTTOM, BOTTOM_LEFT,
FULL_OUT, BEYOND, IN
}
private Position[] avaliablePositions = new Position[]{
LEFT, TOP_LEFT, TOP, TOP_RIGHT, RIGHT,
BOTTOM_RIGHT, BOTTOM, BOTTOM_LEFT,
FULL_OUT, BEYOND, IN
};
private Dictionary<Position, List<DependencyObject> objectsForPosition = new Dictionary<Position, List<DependencyObject>>();
private Position GetPositionWithinPageRect(Point startPoint, Point endPoint){
bool inTop = (PageRect.Top <= startPoint.Y);
bool inBottom = (PageRect.Bottom >= endPoint.Y);
bool inLeft = (PageRect.Left <= startPoint.X);
bool inRight = (PageRect.Right >= endPoint.X);
bool beyond = (PageRect.Top >= endPoint.Y)
|| (PageRect.Bottom <= startPoint.Y)
|| (PageRect.Left >= endPoint.X)
|| (PageRect.Right <= startPoint.X);
if(inTop && inLeft && inRight && inBottom){
return Position.IN;
}
if (startPoint.X == 0 && startPoint.Y == 0 || !beyond && !inTop && !inLeft && !inRight && !inBottom)
return Position.FULL_OUT;
if(!beyond){
if(!inTop){
if(!inRight){
return Position.TOP_RIGHT;
}
if(!inLeft){
return Position.TOP_LEFT;
}
return Position.TOP;
}
if(!inBottom){
if(!inRight){
return Position.BOTTOM_RIGHT;
}
if(!inLeft){
return Position.BOTTOM_LEFT;
}
return Position.BOTTOM;
}
if(!inRight){
return Position.RIGHT;
}
if(!inLeft){
return Position.LEFT;
}
}else{
return Position.BEYOND;
}
}
private bool ForeachChildIn(DependencyObject myPage, IList<DependencyObject> childList)
{
var result = false;
//move this on you ctor
for(int i = 0; i < avaliablePositions.Length; ++i){
objectsForPosition.Add(avaliablePositions[i], new List<DependencyObject>());
}
foreach (var item in childList)
{
var myVisual = item as Visual;
if(myVisual == null) continue;
Point relativeSartPoint = myVisual.TransformToAncestor((Visual)myPage)
.Transform(new Point(0, 0));
var aw = (double)myVisual.GetValue(FrameworkElement.ActualWidthProperty);
var ah = (double)myVisual.GetValue(FrameworkElement.ActualHeightProperty);
Point relativeEndPoint = new Point(relativeSartPoint.X + aw, relativeSartPoint.Y + ah);
Position position = GetPositionWithinPageRect(relativeSartPoint, relativeEndPoint);
if(position != Position.IN){
if (!ForeachChildIn(item, item.getChilds().ToList())){
if ((Visibility)item.GetValue(FrameworkElement.VisibilityProperty) == Visibility.Visible){
if (item is Control)
((Control)item).Background = Brushes.LimeGreen;
else if (item is Panel)
((Panel)item).Background = Brushes.LimeGreen;
}
}
return false;
}else{
objectsForPosition[position].add(iteContext
StackExchange Code Review Q#61768, answer score: 12
Revisions (0)
No revisions yet.