|
Using a windows app to batch run a DOS program.
Need to wait until the DOS app has finished before going on with the next file.
Using:
Process.Start(thecommand, thearguments);
System.Diagnostics.Process p = System.Diagnostics.Process.GetCurrentProcess();
while (!p.HasExited)
{
aaa=5;
}
but it seems to be waiting for my windows app to end, not the DOS app. I can see my DOS app running in a DOS window, and it closes when finished (it takes about a minute to run), but my app sits there in the while loop. I put a breakpoint at aaa = 5; and the app definitely sits in that loop.
|
|
|
|
|
Uh, that's because you're assigning p to the current process, i.e. the one you are running! I think you want
Process p = Process.Start(thecommand, thearguments);
|
|
|
|
|
I must have misunderstood the examples in the answers to other similar questions!!
Working now.
|
|
|
|
|
What is better in terms of good coding practices:
protected virtual void RenderFilledHead(DrawingContext drawingContext)
{
double y = NoteVerticalPosition;
double yRadius = HeadYRadius * 1.1;
double xRadius = HeadXRadius * 1.2;
drawingContext.DrawEllipse(Brushes.Black, NoteHeadPen, new Point(xRadius / 2 + XOffset + 4.5, y), xRadius - 1, yRadius);
}
protected virtual void RenderEmptyHead(DrawingContext drawingContext)
{
double y = NoteVerticalPosition;
double yRadius = HeadYRadius * 1;
double xRadius = HeadXRadius * 1;
drawingContext.DrawEllipse(Brushes.Transparent, NoteBeamPen, new Point(xRadius / 2 + XOffset + 4.5, y), xRadius - 1, yRadius);
}
XOR
protected virtual void RenderHead(DrawingContext drawingContext, NoteHeadTypes type)
{
double y = NoteVerticalPosition;
double xRadius=0, yRadius=0;
switch (type)
{
case NoteHeadTypes.Filled:
yRadius = HeadYRadius * 1.1;
xRadius = HeadXRadius * 1.2;
drawingContext.DrawEllipse(Brushes.Black, NoteHeadPen, new Point(xRadius / 2 + XOffset + 4.5, y), xRadius - 1, yRadius)
break;
case NoteHeadTypes.Empty:
yRadius = HeadYRadius;
xRadius = HeadXRadius;
drawingContext.DrawEllipse(Brushes.Transparent, NoteBeamPen, new Point(xRadius / 2 + XOffset + 4.5, y), xRadius - 1, yRadius);
default:
break;
}
}
NOTE 1: there won't be any more HeadTypes probably.
NODE 2: No, it is not about empty or filled human heads, it's about musical notation 
Greetings - Jacek
EDIT: added "*1" to emphasize that it is a factor as good as '1.2'
|
|
|
|
|
Probably a matter of taste, but I like the second one better.
That shows that apart from different radiusses you are dealing with the same thing. That's harder to see with the first approach.
V.
|
|
|
|
|
What about the following:
protected virtual void RenderHead(DrawingContext drawingContext, NoteHeadTypes type)
{
double y = NoteVerticalPosition;
double xRadius=HeadXRadius;
double yRadius=HeadYRadius;
Pen myPen = NoteBeamPen;
Brush myBrush = Brushes.Transparent;
switch (type)
{
case NoteHeadTypes.Filled:
yRadius *= 1.1;
xRadius *= 1.2;
myPen = NoteHeadPen;
myBrush = Brushes.Black;
break;
default:
break;
}
drawingContext.DrawEllipse(myBrush, myPen, new Point(xRadius / 2 + XOffset + 4.5, y), xRadius - 1, yRadius);
}[Edited]myPen wasn't allocated via a constructor, so it shouldn't be disposed, and I'd missed the alternate brush.
|
|
|
|
|
Sniped!
I think you actually don't want to dispose the pen in this case, though, because it's being read out of a global variable which is presumably caching it.
(Also, the brush is different for the two cases, like the pen.)
|
|
|
|
|
I missed the brushes case, and you're right - the Pen shouldn't be disposed. For some reason I thought I'd newed it.
|
|
|
|
|
I don't like it because it prefers Brushes.Transparent over Brushes.Black. Both brushes are equal. Here, if switch goes to the default case, then Brushes.Transparent would be used despite there isn't any reason to use it and not Brushes.Pink. The same with radius factors -- factor '1.2' isn't more special than '1'.
Greetings - Jacek
|
|
|
|
|
Fair point, but you could simply exit out in the default case (I've never been a fan of default in switch statements - it's always seemed that it should be for the error case).
|
|
|
|
|
Pete O'Hanlon wrote: Fair point, but you could simply exit out in the default case
Oh these sexy quotation boxes.
Well, it still visually suggests that Transparent is a default value, because brush is assigned twice. Oh and redutant assignment == wasted CPU cycles.
Greetings - Jacek
|
|
|
|
|
I rather agree – see my post where I used the default for a throw. When you're switching on an enum it's usual that you have defined behaviour for all cases and anything unexpected should throw an exception.
|
|
|
|
|
I prefer the second, but I'd take the ellipse drawing out of the switch, since the logic is (to me) 'set up parameters depending on head type, then draw it' and the last part is common. So I'd do:
protected virtual void RenderHead(DrawingContext drawingContext, NoteHeadTypes type)
{
double y = NoteVerticalPosition;
double xRadiusFactor, yRadiusFactor;
Brush brush;
Pen pen;
switch (type)
{
case NoteHeadTypes.Filled:
yRadiusFactor = 1.1;
xRadiusFactor = 1.2;
brush = Brushes.Black;
pen = NoteHeadPen;
break;
case NoteHeadTypes.Empty:
yRadiusFactor = xRadiusFactor = 1;
brush = Brushes.Transparent;
pen = NoteBeamPen;
break;
default:
throw new ArgumentException("Unknown head type");
}
double xRadius = HeadXRadius * xRadiusFactor,
yRadius = HeadYRadius * yRadiusFactor;
drawingContext.DrawEllipse(brush, pen, xRadius / 2 + XOffset + 4.5, y, xRadius - 1, yRadius);
}
|
|
|
|
|
This is good, I like explicitness of all assignments, like brush, pen and xyRadiusFactor. DrawEllipse may change individually for each method, but this solution makes future refactoring easy.
Greetings - Jacek
|
|
|
|
|
I'd personally go for option 1, I'm of the opinion that switches are evil, but everytime I express this view I get downvoted. Rather than explain why here, have a look at SwitchStatementsSmell[^].
If you can't alter your class design much, you could consider this option (removing duplication under the "DRY" principle) :
virtual void RenderHead(DrawingContext drawingContext,
Brush brush = Brushes.Transparent,
Pen pen = NoteBeamPen,
decimal xScale =1,
decimal yScale =1)
{
double y = NoteVerticalPosition;
double yRadius = HeadYRadius * yScale;
double xRadius = HeadXRadius * xScale;
drawingContext.DrawEllipse(brush, pen, new Point(xRadius / 2 + XOffset + 4.5, y), xRadius - 1, yRadius);
}
protected virtual void RenderHead(DrawingContext drawingContext, NoteHeadTypes type)
{
if(type==NoteHeadTypes.Filled)
RenderHead(drawingContext, Brushes.Black, NoteHeadPen, 1.2, 1.1);
else if(type==NoteHeadTypes.Empty)
RenderHead(drawingContext);
}
Note this code only works with .net 4 (for brevity), but the named arguments can easily be refactored back.
|
|
|
|
|
I prefer to remove the switch smell, but sometimes it's overkill. In all things there should be balance.
|
|
|
|
|
Sorry, but it's all bad.
1. First of all, it doesn't compile because neither NoteBeamPen nor Brushes.X are constant.
2. It is not clear what 'factors' RenderHead(drawingContext) are. I don't see why factor '1' should be 'more default' than, e.g. 0 or 10 or 1.5 or whatever.
3. Additionally, drawingContext.DrawEllipse is not actually redutant code. It can change to 'DrawGeometry' or other stuff in future, and then it would require refactoring if different geometry was used in these methods. And, using your solution refactoring wouldn't be straightforward.
4. You have replaced switch with ifs which doesn't actually change devilness.
Greetings - Jacek
modified on Tuesday, June 28, 2011 7:11 AM
|
|
|
|
|
Jacek Gajek wrote: 1. First of all, it doesn't compile because neither NoteBeamPen nor Brushes.X are constant.
I didn't copile it as I don't know which namespaces you are using, just describing the princple. Perhaps I should have made this clear.
Jacek Gajek wrote: 2. It is not clear what 'factors' RenderHead(drawingContext) are. I
don't see why factor '1' should be 'more default' than, e.g. 0 or 10 or 1.5 or
whatever.
This defaults to the "empty" version, but that is all they are defaults. You could default to whatever you feel safest, but defaulting to the one that scales at 1 makes the most sense to me.
For both the points above,you can drop the defaults totally and pass these in makes you feel any better, without altering the key point I made.
Jacek Gajek wrote: Additionally, drawingContext.DrawEllipse is not actually redutant code. It can
change to 'DrawGeometry' or other stuff in future, and then it would require
refactoring if different geometry was used in these methods.
I didn't say that was redundant, just that you have repeated code, as other posters have pointed this out to you too. Further refactoring is unlikely be a huge chore, and you'd only need to do code on one place most probably. In any case I also question the wisdom of future-proofing against "geometry changes" unless they are reasonably certain to change. On this last point, if these are likely to change, how we we be able to tell, answer: we can't wihtout you letting us know your requirements.
|
|
|
|
|
Keith Barrow wrote:
I didn't copile it as I don't know which namespaces you are using, just describing the princple. Perhaps I should have made this clear.
Well they cannot be constant unless it was a 'null' constant.
I do not agree with an idea of defining "default" values, even if their usage is optional. In my opinion, method signature should force developer to explicitely assign parameters. And it is not all about providing more precise requirements. But well, you are impressively good in defending your ideas, 5 for this 
Greetings - Jacek
|
|
|
|
|
Counter-5 for being a good sport!
|
|
|
|
|
If switch statements 'smell', then
if(thing == option1) ...
else if(thing == option2) ...
... also smells, and (unless it's a type for which switch is not supported in the language) it is worse than doing the switch explicitly because it's harder to see that that's what you're doing.
The rule of thumb is to do with duplication. If he's switching on this enum in several places, perhaps the class hierarchy should be adjusted so that it is not an enum, it is a class tree with a Render method (and methods for the other operations that are being switched on). There isn't really any evidence to suggest that that is the case in the original post, though, and it's somewhat outside the scope of the question that was asked.
|
|
|
|
|
BobJanova wrote: also smells
Agreed, but where I'd disagree is here:
BobJanova wrote: because it's harder to see that that's what you're doing.
I'd say having the condition visible at each branch is more clear, but that is a matter of opinion.
BobJanova wrote: If he's switching on this enum in several places, perhaps the class hierarchy should be adjusted so that it is not an enum
Yes, and the article I pointed him to makes this very point. As I said, he might not have the luxury of altering that much (the graphics classes are riddled with enums, forcing attendant switches), so I gave him another option.
|
|
|
|
|
About the second point: the reason the switch is clearer is because you can see immediately when you see the word 'switch' that it is going to be a selection based on a single value. With the if/else there's nothing to stop you doing
if(c == ' ') ...
else if(c == '/') ...
else if(c == '^' && accountBalance > 50) ...
else if(c == '&') ...
... so you have to read each line carefully to know what the logic is actually doing.
|
|
|
|
|
I demonstrate using Action to perform functionality instead of using a switch (or an if/else condition) in this[^] blog post.
|
|
|
|
|
BobJanova wrote: class tree with a Render method
I have considered it but it has prons and cons too -- see my answer to "SledgeHammer01"[^].
Greetings - Jacek
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
