Log in

View Full Version : MeGUI development


Pages : 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 [27] 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92

Doom9
17th January 2006, 13:26
@dimzon: I've added you as a developer. However, when it comes to refactoring, the same rules still apply: ask me before you do anything. As far as the audio configuration dialogs go, you are cleared to modify those.. everybody else hands off please. Keep in mind though that I'd not yet like to lose the besweet encoding (it should be able to switch.. even if only by a non GUI exposed bool in the MeGUIsettings), and make your new audio encoder integrate with the current architecture.

dimzon
17th January 2006, 13:30
However, when it comes to refactoring, the same rules still apply: ask me before you do anything.
Ok. I will perform Audio Dialogs Refactoring & a little MeGuiSettings changes & cleanups
Keep in mind though that I'd not yet like to lose the besweet encoding (it should be able to switch.. even if only by a non GUI exposed bool in the MeGUIsettings), and make your new audio encoder integrate with the current architecture.
Ok.

dimzon
17th January 2006, 13:46
2 All
How does You commit changes to CVS?
I'm trying to use "Commit..."

In C:\Documents and Settings\DAlexandrov\My Documents\Visual Studio 2005\Projects\MeGUI\MeGUI-src.CVS: "C:\Program Files\TortoiseCVS\cvs.exe" "-q" "commit" "-m" "3 additional properties for faac, neroaac and lame" "MeGUISettings.cs"
CVSROOT=:ext:dimzon@cvs.sourceforge.net:/cvsroot/megui

cvs: rcs.c:4188: RCS_checkout: Assertion `rev == ((void *)0) || ((*__ctype_b_loc ())[(int) (((unsigned char) *rev))] & (unsigned short int) _ISdigit)' failed.
cvs [commit aborted]: received abort signal

Error, CVS operation failed

Doom9
17th January 2006, 13:55
never happend to you in 5 years? It happened for my second commit. Here's what you should not do: do a fresh CVS checkout, overwrite those files with your changed ones, then commit... that's what I did and it's no fun. I'm afraid it looks like you're in for a manual treat (I suggest a fresh checkout, compare files and merge your changes). But perhaps somebody knows more.. either way it looks like a bugreport for tortoise would be in order.

dimzon
17th January 2006, 14:02
never happend to you in 5 years?
Microsoft Visual Source Safe works fine!

dimzon
17th January 2006, 14:16
2 All
How does You commit changes to CVS?
I'm trying to use "Commit..."

In C:\Documents and Settings\DAlexandrov\My Documents\Visual Studio 2005\Projects\MeGUI\MeGUI-src.CVS: "C:\Program Files\TortoiseCVS\cvs.exe" "-q" "commit" "-m" "3 additional properties for faac, neroaac and lame" "MeGUISettings.cs"
CVSROOT=:ext:dimzon@cvs.sourceforge.net:/cvsroot/megui

cvs: rcs.c:4188: RCS_checkout: Assertion `rev == ((void *)0) || ((*__ctype_b_loc ())[(int) (((unsigned char) *rev))] & (unsigned short int) _ISdigit)' failed.
cvs [commit aborted]: received abort signal

Error, CVS operation failed

Ok. Just do not perform "unedit" command before

dimzon
17th January 2006, 15:56
Doom9, I have some questions/proposals:

1) Show command line @ AudioSettingsDialog
What I must output there (all processing except encoding is done by AviSynth script)

2) Multichannel code. Current solution consist of 2 radio buttons (stereo && 5.1) & 1 combo box (stereo, dpl, dpl2). I propose 1 combobox instead:
Name: Multichannel Processing
Values:

Keep Original Channel Count
Convert To Mono
Downmix to Stereo
DPL Downmix
DPL II Downmix

Doom9
17th January 2006, 16:02
2) Multichannel code. Current solution consist of 2 radio buttons (stereo && 5.1) & 1 combo box (stereo, dpl, dpl2). I propose 1 combobox instead:
Name: Multichannel ProcessingThat works for your dialog, but not the BeSweet one.. it needs to be told if the source is 2.0 is 5.1 (well, not exactly told.. but you need the construct a commandline that works for either 5.1 sources and keeps 'em that way, or works for 5.1 with downmix/2.0 sources). Changing these options in the existing dialog would make besweet encoding impossible as besweet neets to be told if it has to operate in 2.0 or 5.1 mode.. even though it would be perfectly capable of figuring that out on its own.

1) Show command line @ AudioSettingsDialog
What I must output there (all processing except encoding is done by AviSynth script) Well, you're still running a commandline encoder somewhere, are you not? So the commandline for it would be appropriate.

dimzon
17th January 2006, 16:46
That works for your dialog, but not the BeSweet one.. it needs to be told if the source is 2.0 is 5.1 (well, not exactly told.. but you need the construct a commandline that works for either 5.1 sources and keeps 'em that way, or works for 5.1 with downmix/2.0 sources). Changing these options in the existing dialog would make besweet encoding impossible as besweet neets to be told if it has to operate in 2.0 or 5.1 mode.. even though it would be perfectly capable of figuring that out on its own.
I'm planning to write managed wrapper around mediaInfo.dll to obtain original channel count in beSweet case..

Well, you're still running a commandline encoder somewhere, are you not? So the commandline for it would be appropriate.
Ok.

Doom9
17th January 2006, 16:54
I'm planning to write managed wrapper around mediaInfo.dll to obtain original channel count in beSweet case..Don't bother.. once your avisynth based encoding works and we are sure we're not going to lose functionality by removing the besweet part, the besweet part will be removed. It makes little sense to invest additional effort into something that is going to be gone sooner or later.

In addition, it wouldn't really always help.. consider the case where somebody is configuring audio first (e.g. creating a profile), and then loading the actual file. So in the end you'd have to override the commandline at the beginning of encoding to adapt in function of the source properties..

dimzon
17th January 2006, 17:03
2 all Developers

New file @ CVS - EmumProxy.cs

this is helper class to bind Enum's to UI with friendly names
use sample is commented out @ the end of file (take a look)
ps. May be not very optimal (private implementation is subject of change)


public enum DownmixMode
{
[EnumTitle("Keep Original Channels")]
DoNothong,
[EnumTitle("Convert to mono")]
ToMono,
...
[EnumTitle("Dolby Pro Logic II")]
ToDPL2
}


MyCombo.Options.AddRange(EnumProxy.CreateArray(new object[]{DownmixMode.DoNothong,...,DownmixMode.ToDPL2}))

Doom9
17th January 2006, 17:06
Why not just use ints? That works fine for the ProcessPriority enum (you can find it in Job.cs I think).. it is mapped to a dropdown.

Hmm.. on second thought, is the point to have the enum names in text?

dimzon
17th January 2006, 17:10
Don't bother.. once your avisynth based encoding works and we are sure we're not going to lose functionality by removing the besweet part, the besweet part will be removed.
Isn't existing technology implementation (BeHappy) enought to be sure? Maybe I will wait a little until You inspect BeHappy more careful?

So in the end you'd have to override the commandline at the beginning of encoding to adapt in function of the source properties..
I'm a really sorry, I do not understand You at all! Command line is _private_ implementation detail. You does not like about Job can create it's own encoder but You hold specific information for concrete encoder implementation!

dimzon
17th January 2006, 17:15
Why not just use ints? That works fine for the ProcessPriority enum (you can find it in Job.cs I think).. it is mapped to a dropdown.
If You change your enum order You must syncronize your changes with ComboBox.


Hmm.. on second thought, is the point to have the enum names in text?
Yes! You can specify friendly names for enum. This is MAIN point.

stax76
17th January 2006, 17:22
public enum DownmixMode
{
[EnumTitle("Keep Original Channels")]
DoNothong,
[EnumTitle("Convert to mono")]
ToMono,
...
[EnumTitle("Dolby Pro Logic II")]
ToDPL2

MyCombo.Options.AddRange(EnumProxy.CreateArray(new object[]{DownmixMode.DoNothong,...,DownmixMode.ToDPL2}))
}

That's nifty, exactly my way of doing things though I would automate much further. Is EnumTitle a .NET class? If so then I've missed it.

dimzon
17th January 2006, 17:28
Is EnumTitle a .NET class? If so then I've missed it.
:D
Yes, it's .NET class (but it's not a part of .NET Framework). Definitly - every MeGUI class is .NET class :)

EnumTitle is my custom attribute defined @ EnumProxy.cs

stax76
17th January 2006, 18:09
You could populate the list control by passing the enum type to a method, other than that my code is very similar:


Public Class ListBag(Of T)
Public Sub New(ByVal text As String, ByVal value As T)
Me.Text = text
Me.Value = value
End Sub

Private TextValue As String

Public Property Text() As String
Get
Return TextValue
End Get
Set(ByVal Value As String)
TextValue = Value
End Set
End Property

Private ValueValue As T

Public Property Value() As T
Get
Return ValueValue
End Get
Set(ByVal Value As T)
ValueValue = Value
End Set
End Property

Public Shared Sub SelectItem(ByVal cb As ComboBox, ByVal value As T)
Dim selectItem As Object = Nothing

For Each i As ListBag(Of T) In cb.Items
If i.Value.Equals(value) Then
selectItem = i
End If
Next

If Not selectItem Is Nothing Then
cb.SelectedItem = selectItem
End If
End Sub

Public Shared Function GetValue(ByVal o As Object) As T
Return DirectCast(DirectCast(o, ListBag(Of T)).Value, T)
End Function

Public Shared Sub PopulateList(ByVal l As IList)
For Each i As T In System.Enum.GetValues(GetType(T))
l.Add(New ListBag(Of T)(NameAttribute.GetEnumName(Of T)(i), i))
Next
End Sub

Public Overrides Function ToString() As String
Return Text
End Function
End Class

<AttributeUsage(AttributeTargets.All)> _
Public Class NameAttribute
Inherits Attribute

Public Sub New(ByVal name As String)
ValueValue = name
End Sub

Private ValueValue As String

Public ReadOnly Property Value() As String
Get
Return ValueValue
End Get
End Property

Public Shared Function GetEnumName(Of T)(ByVal value As T) As String
For Each i As FieldInfo In value.GetType.GetFields
If i.GetValue(value).Equals(value) Then
Dim attributes As Object() = i.GetCustomAttributes(False)

For Each i2 As Object In attributes
If i2.GetType Is GetType(NameAttribute) Then
Return DirectCast(i2, NameAttribute).Value
End If
Next

Return i.Name
End If
Next

Throw New Exception
End Function
End Class

dimzon
17th January 2006, 20:09
New base AudioConfiguration prototype - proposal
http://img388.imageshack.us/img388/5016/untitled1lt.png

FFWD
17th January 2006, 20:26
Dimzon, I see downmix to PLII in your screenshot. So is there a way to get a Dolby Pro Logic II AAC from a 5.1 AC3? Does it also work with HE-AAC files? And more important; does the decoding actually work? (my Logitech Z-5500 supports Pro Logic II)

Doom9
17th January 2006, 21:00
@dimzon: I think the load defaults button also couldn't hurt.. it was requested for video but it makes sense for audio as well. Other than that, it looks nice. Obivously this is only the prototype for AviSynth based processing, but since that's the direction we're heading in.. it looks how it should be.

berrinam
18th January 2006, 02:24
0.2.3.2032 18 Jan 2006
Bugfix: Scripts served by scriptserver now work (that's a good thing :D)
Bugfix: Fixed problems with comma locales in string->double conversions in source detection.

Is there a way to specify the locale across the entire GUI or do some other solution, as this problem keeps coming up?

berrinam
18th January 2006, 04:03
0.2.3.2033 18 Jan 2006
Bugfix: AviSynth errors are now displayed in the preview window (changes made by looking at AVIFile code).

EDIT: This code handles YV12 input differently from other input (because YV12 needs to be converted, whereas DIB is fine). As a result, it is possible to tell whether the AviSynth output is YV12, and if it isn't, warn the user that it won't work. What I'm saying is that it is possible for MeGUI to catch the 'unsupported input colorspace' error before it occurs. It could even suggest adding converttoyv12 if it isn't actually an error.

Richard Berg
18th January 2006, 04:23
Is there a way to specify the locale across the entire GUI or do some other solution, as this problem keeps coming up?
Not really. Programmers should know that anytime you do string manipulation, you need to decide whether current locale vs. invariant locale is appropriate. This is very easy compared with the problems I encounter with localized builds & non-unicode encodings :)

Doom9
18th January 2006, 09:30
you need to decide whether current locale vs. invariant locale is appropriate.Invariant doesn't seem to cut it.. I first tried that in the getFrameNumber methods in the video encoders.. it still didn't work on those comma locales, so I forced it to en-us.

dimzon
18th January 2006, 11:42
Dear Doom9
Can you answer on this post http://forum.doom9.org/showthread.php?p=769907#post769907

Doom9
18th January 2006, 11:50
Isn't existing technology implementation (BeHappy) enought to be sure?Looking at your configuration template I'd say yes.. as long as you can cover everything in the BESweet configuration groupbox we won't have any problems.

I'm a really sorry, I do not understand You at all! Command line is _private_ implementation detail. You does not like about Job can create it's own encoder but You hold specific information for concrete encoder implementation!Make that a "currently a job holds the commandline". However, that is going to change.. in fact as it is now it doesn't make a whole lot of sense and the commandline should only be generated from the settings when encoding is to take place. That'll even permit an encoder change at the last moment (for instance mencoder <-> x264). I already had it like that in the past.. there was a reason why I changed it but I can't quite remember right now why it was.. just hoping I won't break anything if I go back to the "encoder generates commandline" approach.
As it is now, it's already possible that you change the path of the encoder after creating the job and you can still encode (except for dgindex.. it still includes the binary name in the commandline).. so we're already halfway there.. but the other half mile is to be walked...

dimzon
18th January 2006, 11:55
Looking at your configuration template I'd say yes.. as long as you can cover everything in the BESweet configuration groupbox we won't have any problems.
Does it mean I can remove BeSweet-related code?

in fact as it is now it doesn't make a whole lot of sense and the commandline should only be generated from the settings when encoding is to take place
Does it mean we does'nt need "Show commandline" option @ configuration dialog?

Doom9
18th January 2006, 12:00
Does it mean I can remove BeSweet-related code?No, as I said.. keep all the code there.. I'll remove it in time. Just don't bother with anything BeSweet related.. write your own dialogs and your own encoder derived from AudioEncoder.

Does it mean we does'nt need "Show commandline" option @ configuration dialog?No, I think that's still good.. it also helps in development (you immediately see what the commandline will be.. the only changes that can come are bitrate related when the bitrate is being automatically calculated).

dimzon
18th January 2006, 12:00
Audio via AviSynth disadvantage
Current implementation (reading audio data via VfW) can't provide detailed information about script error.
Workaround: to work with avisynth.dll directly (without VfW). Unfortunally it's impossible bcz AviSynth API can be accessed only via C++. I'm planning to write wrapper in Managed C++ to provide full power of AviSynth functionality to .NET world...

dimzon
18th January 2006, 12:09
No, as I said.. keep all the code there.. I'll remove it in time.
As you can see I'm altering AudioSettings class (adding new items in emunerations etc). So I cant "Just do not touch BeSweet code" bcz it's break compilation. There are 2 way - perform appropriate adjustments in BeSeet-related code or just remove it @ forget it...

No, I think that's still good.. it also helps in development (you immediately see what the commandline will be.. the only changes that can come are bitrate related when the bitrate is being automatically calculated).
Does it mean I must invoke command-line creation logic twice (ones in audioSettingsDialog, ones before encoding startup)?
So does it mean I can do "getCommandLine()" - a virtual method for AudioSettings?

Doom9
18th January 2006, 12:50
Current implementation (reading audio data via VfW) can't provide detailed information about script error.Neither can any of the video tools.. that's why if I can't open the script via VfW, I do not allow encoding to proceed. I think your audio encoder should do something similar.. open it, see if the properties make sense, and if not, abort and ask the user to play the script in a media player.

As you can see I'm altering AudioSettings class (adding new items in emunerations etc). So I cant "Just do not touch BeSweet code" bcz it's break compilation.So use the variables you're going to add for the avisynth part, and leave the other ones in.. that means a bloated AudioSettings class, but that can be streamlined when we dump the besweet part.

Does it mean I must invoke command-line creation logic twice (ones in audioSettingsDialog, ones before encoding startup)? Yes.

So does it mean I can do "getCommandLine()" - a virtual method for AudioSettings?Since the encoder commandline is taken from the MeGUISettings class, that would mean your derived AudioSettings classes would need to be aware of the settings.. I don't think that's such a good idea. I like the current approch where the encoder and the GUI class have a reference to the settings better.. the job and its settings shouldn't be aware of settings which mostly don't affect them, should they?

dimzon
18th January 2006, 13:53
Since the encoder commandline is taken from the MeGUISettings class
Encoder executable path, not commandline. It's possible to generate commandline arguments without full path (and add real path later).

Doom9
18th January 2006, 14:01
Encoder executable path, not commandline.Yeah, sorry about that. I think we can do away with the necessity of also showing the executable path in the codec configuration, but having the rest (generated of course) is really nice.

Sharktooth
18th January 2006, 15:00
Sorry but i have an infection of the inner hear (dont know the english medical term) that causes me vertigos and other problems.
So i cant stay too much in front of the PC and cant read too much (i cant focus on text).
Bear with me... i'll be back.

dimzon
18th January 2006, 18:01
@Doom9

public interface IJobProcessor
{
/// <summary>
/// sets up encoding
/// </summary
/// <param name="job">the job to be processed</param>
/// <param name="error">output for any errors that might ocurr during this method</param>
/// <returns>true if the setup has succeeded, false if it has not</returns>
bool setup(Job job, out string error);

.NET Has PERFECT error reporting mechanism: Exceptions && Exceptions Hadling. Why does You try propose such ugly method?

Doom9
18th January 2006, 18:17
.NET Has PERFECT error reporting mechanism: Exceptions && Exceptions Hadling. Why does You try propose such ugly method?Because I don't like exceptions? It makes you have to wrap everything into a try/catch block.. and you shouldn't use exceptions for normal program flow.. For instance, if setup returns false because the encoder is not found, it would be a resource waste to resort to an exception for that. There are quite a few instances when an error is reported back that corresponds to regular program flow and thus shouldn't use an exception.

stax76
18th January 2006, 18:27
and you shouldn't use exceptions for normal program flow.

Are you sure? Never tried it but recently I thought it could be a cheap way to abort.

Doom9
18th January 2006, 18:29
Are you sure?It says so in all the literature I should get around to reading for my certification. And Exceptions are resource intensive (I don't recall the exact specifics... but throwing an exception slows you down quite a bit as opposed to using a return variable).

Richard Berg
18th January 2006, 19:51
I don't agree with dimzon in this specific example, but he has a good point. Having to pass error messages around as parameters -- making every caller store or otherwise handle them -- is usually unnecessary work for the programmer. The most helpful feature of exceptions isn't handling them, it's the option not to. If a method isn't equipped to handle some conditions, flow simply passes to the previous caller.

Throwing an exception is kinda expensive: it causes a hardware interrupt that has to be bubbled all the way up from the kernel. Figure a few thousand CPU cycles. But performance is totally irrelevant for asynchronous GUI apps like MeGUI...so long as you're not throwing exceptions in a tight loop you'd never notice.

A decent rule of thumb: exceptions should roughly correspond with problems that are important enough to show the user an error message. If a program throws & catches often without bothering the user, it's probably overusing exceptions. (It's crazy what you sometimes see if you run badly written software it under a debugger that's set to catch all native exceptions...)

With IJobProcessor.Setup, I think using an error parameter is appropriate. Any method that calls Setup should know what to do if it receives an error string. But it should be simpler: instead of having a bool and a string, just return a string; if it's not null, it's an error.

Mutant_Fruit
19th January 2006, 00:01
From what i've been told and what i've read:

Exceptions shouldn't be used to handle normal workflow. If a value you're using has a good chance of NOT being what you expect it to be, use an if statement to see if it's valid. Don't use try/catch to find out. Exceptions are what they say.... Exceptions. They should rarely (if ever) happen.

I'd agree with Richard Berg about not needing a bool AND a string. Thats about it from me... *looks for the 'i am a n00b' poster*

Richard Berg
19th January 2006, 01:12
I wouldn't take any of these rules too far. Exceptions and return values are just tools; good programmers simply choose the best one for the job. Worrying that you'll trigger an exception often enough that someone might call it "normal" is premature optimization, which we know is the root of all evil.

In this case, forcing the immediate caller of IJobProcessor.Setup to always test the return value works ok with our design, so no exception necessary. If IJobProcessor.Setup had dozens of possible entry points, using an exception would be better because it guarantees that someone in the call stack will do something about it.

dimzon
19th January 2006, 12:57
2 Doom9, Richard Berg, Mutant_Fruit
and you shouldn't use exceptions for normal program flow..
<offtopic>
Does and you know how does woking FileMapping/VirtualMemory/PageFile mechanism? Whe you trying to addreess to non-existing page in RAM processor will generate hardware interrupt (exception). Windows catch this interrupt, look for this page in PageFile, load it to the memory. So exceptions (sometimes) is a really fine to be used in normal program flow...
Another sample - for ASP.NET programmers. When You invoke Response.End exception is trown, really, try this code

try
{
Response.End();
}
catch(Exception e)
{
Response.Write(e.ToString());
}

And yet another sample for threadind: When you invoke thread.Abort() ThreadAbordet exeption is throwing
</offtopic>

But my sample IS NOT A NORMAL PROGRAM FLOW.
If JobProcessor can't setup it mean:

You trying to setup JobProcessor for unsupported job
Job configuration is invalid
JobProcessor configuration is invalid
Another shit happens

All options is exceptions, isn't it...

With IJobProcessor.Setup, I think using an error parameter is appropriate. Any method that calls Setup should know what to do if it receives an error string.
I really don't think You will have multiple places to call IJobProcessor.Setup, but if You really want to do it end you does'nt want to wrap your code using try/catch @ every place you can use PATTERN-ADAPTER for it:

public interface IJobProcessor
{
void Setup(Job job);
}

public class JobProcessorAdapter
{
private IJobProcessor _proc;
public JobProcessorAdapter(IJobProcessor p)
{
_proc = p;
}

public bool setup(Job job, out string error)
{
try
{
_proc.Setup(job);
return true;
}
catch(Exception e)
{
error = e.Message;
return false;
}

}
}



2 All developers

I have found another ugly code:

MyForm f = new MyForm();
if(f.ShowDialog()==DialogResult.OK)
{
// Do something
}


Proper way:

using(MyForm f = new MyForm())
{
if(f.ShowDialog(this)==DialogResult.OK)
{
// Do something
}
}

Doom9
19th January 2006, 13:30
I have found another ugly code:What is the problem with it? The local method variable will go out of scope as soon as you exit the method and will be garbage collected..

dimzon
19th January 2006, 14:14
What is the problem with it? The local method variable will go out of scope as soon as you exit the method and will be garbage collected..
Form implemets IDisposabe interface. It means it hold unmanaged resources. Unmanaged resources can't be collected via garbage collector.
Really, garbage collecter will be called only @ deficit memory. But you object can hold window handler or other handles, SqlConnections e.t.c.

Read this:
http://msdn.microsoft.com/library/default.asp?url=/msdnmag/issues/1100/GCI/TOC.ASP?frame=true
http://msdn.microsoft.com/library/default.asp?url=/msdnmag/issues/1200/GCI2/TOC.ASP?frame=true
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dndotnet/html/dotnetgcbasics.asp?frame=true

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfsystemidisposableclasstopic.asp
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dncscol/html/deepc10192000.asp

dimzon
19th January 2006, 14:33
Dear Doom9.
MeGUI invokes DGDecode.dll direcly. It causes such problem:
DgDecode.dll must be im MeGUI folder or be avaluable @ PATH enviroment string.
In other hand DGDecode.dll MUST be @ avisynth\plugins folder in order to be able open scripts.
So I want to propose solution - let's open d2v files via AviSynth temporaly script file.

Mutant_Fruit
19th January 2006, 15:00
Whe you trying to addreess to non-existing page in RAM processor will generate hardware interrupt (exception)
Aye. This is what i would classify as an "exception". You've tried doing something that is 'impossible'. You can't access a non-existing page, so rightfully an exception should be thrown. This would be normal program flow.

But in this example assume the string "str_value" usually contains an integer, but could occasionally be null....

try
{
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //the string was null or contained invalid data
}

Thats the wrong way of doing it. The right way is this:

try
{
if(str_value != null)
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //string contained invalid data
}

You check to see if it's null BEFORE trying to convert it. That way exceptions will only happen when an "unexpected" event happens. Being null is an expected event that should be checked for.

dimzon
19th January 2006, 15:06
@Mutant_Fruit
But in this example assume the string "str_value" usually contains an integer, but could occasionally be null..

agreed 100%. But IF You cange this stament to
But in this example assume the string "str_value" MUST contains an integer then
try
{
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //the string was null or contained invalid data
}
is valid.

* You trying to setup JobProcessor for unsupported job
* Job configuration is invalid
* JobProcessor configuration is invalid
* Another shit happens

You MUST call JobProcessor for supported job type only
You MUST provide valid job
You MUST provide valid configuaration

So using exceptions in that case is good, is'nt it?

Doom9
19th January 2006, 15:15
* You trying to setup JobProcessor for unsupported jobCan't happen
* Job configuration is invalidCan't happen
* JobProcessor configuration is invalidCan't happen
* Another shit happensLike?

The only thing that can go wrong in the setup is that the encoder cannot be found.. other than that, nothing can go wrong. It is the job of a jobdispatcher to send jobs to where they belong (yeah I know you hate the "is" type checks.. but they ensure this never ever happens). Similarly, it must not be possible to create invalid jobs.. it's the job of the jobcreator to make sure it returns a valid job, or nothing at all.. incidentally it's done that way.. jobs that would be invalid are not returned, instead you get a null value and it's not added to the queue.

dimzon
19th January 2006, 15:20
JobProcessor configuration is invalid
Can't happen

No, it can. Executable path is JobProcessor configuration.

JobProcessor configuration is invalid
Like?

Don't know. Some shit (maybe) @ some implementation...

Mutant_Fruit
19th January 2006, 19:36
@Mutant_Fruit

agreed 100%. But IF You cange this stament to

"But in this example assume the string "str_value" MUST contains an integer"

then
try
{
int result = Convert.ToInt32(str_value);
}
catch
{
int result = -1; //the string was null or contained invalid data
}
is valid.
Aye. In that case it would be valid. But in my example we knew that at times the input could be null and at times it could contain an int. In that case we should check if it is null BEFORE we try to parse out an int to avoid throwing an exception when the str_value is null. In my example, a null value can be expected and so shouldn't throw an exception.

In your example, you are relying on an int being contained in the string (str_value MUST contain an int). Since the int MUST be there, an exception should be thrown if an int is not there (i.e. the string is null or contains text).

In my example, if the string was null i could return a value indicating a fail result, but i wouldn't use an exception to let the program know that the string was null.

bool ParseOutMyInt(string str_value)
try
{
if(str_Value != null)
result = Convert.ToInt32(str_value);
else
return false; // str_value is null, therefore not parsable

//do rest of code and return true for success
}
catch
{
return false; // it failed somewhere in the code.
}