How to Train Your Programmer

Rajesh_Francis

I just found the following code if (Name == "Ticket") { IngGenInvPs post = new IngGenInvPs(taxBO, "Ticket"); } else if (Name == "Advertisement") { IngGenInvPs post = new IngGenInvPs(taxBO, "Advertisement"); } else if (Name == "Asset") { IngGenInvPs post = new IngGenInvPs(taxBO, "Asset"); } else if (Name == "Telecom") { IngGenInvPs post = new IngGenInvPs(taxBO, "Telecom"); } else { IngGenInvPs post = new IngGenInvPs(taxBO, "Inventory"); } :cool:

OriginalGriff

It's not that bad! Well, yes, you could do it with a switch:

IngGenInvPs post;
switch (Name)
{
case "Ticket":
case "Advertisement":
case "Asset":
case "Telecom":
post = new IngGenInvPs(taxBO, Name);
break;
default:
post = new IngGenInvPs(taxBO, "Inventory");
break;
}

Or preferably, just remove it as all the created variables go out of scope at the end of the block - unless they are being sneaky in the constructor!

Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water

Rajesh_Francis

ok, how about this IngGenInvPs post = new IngGenInvPs(taxBO, Name);

OriginalGriff

And if I prefix that with

Name = "Hello, this is a lot of rubbish!";
IngGenInvPs post = new IngGenInvPs(taxBO, Name);

Does your code give the same result as the original?

Ideological Purity is no substitute for being able to stick your thumb down a pipe to stop the water

Nagy Vilmos

psst! Use the markup <pre> to make your code purdy.

Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

bosedk

Not a shame IMO. Having a bunch of if-else or a switch-case setup is the simplest and most straight forward to do this for five to six cases. The conditionals could be moved inside the constructor to make the code more readable though.

Manfred Rudolf Bihy

bosedk wrote:

The conditionals could be moved inside the constructor

I hope not! This would only be advisable if the differentiation between the 5 strings were something inherent to that class, but then OP should have used an enumeration instead of strings. Regards, Manfred

"With sufficient thrust, pigs fly just fine."

Ross Callon, The Twelve Networking Truths, RFC1925

BobJanova

It probably does in the context of the requirement. If you require fixing to a small set, firstly it should be an enum not a string input, and secondly if there is a good reason to allow free text input it's still better to have a simple check along the lines of

if(!ValidNames.Contains(Name)) throw ...;

... and then just construct the object from the name.

bosedk

Manfred R. Bihy wrote:

I hope not!

Aah, yes. I now see why not!

Manfred R. Bihy wrote:

but then OP should have used an enumeration instead of strings.

the text might be coming from user, who knows, so converting text to enums and checking against the enums would be additional no value added steps for the simple scenario. But without proper context, the code seems alright.

0bx

"Training" is usually something you can only do in your free time, during the times you should be sleeping. At work you have to deliver stuff. If you have a limited amount of time to deliver something that works and you're not yet quite experienced (and without any formal education on software development), then this sort of stuff happens. It's not pretty but it works... for now... :^)

Giraffes are not real.

Lutoslaw

So.. is the actual logic implemented in a costructor? post is out of scope for the rest of the world unless the code you have posted is incomplete.

Greetings - Jacek

RobCroll

Switch statements in Java didn't support strings as arguments until recently. Objective C still doesn't. So no training required :laugh:

"You get that on the big jobs."

Rajesh Anuhya

:thumbsup: +5

My Tip/Trick[^]

Nagy Vilmos

In Java, I'd use an Enum[^].

Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

Bernhard Hiller

Below that if-else if-else block, there must be some more lines like:

ResultObject res = post.DoSomething();

And someone complains then: "Why does it not compile!!!"

Jeroen De Dauw

He forgot to wrap each line in a try catch block! o_O

Jeroen De Dauw (blog | Twitter | Identi.ca)

johannesnestler

I have done things like this in the past, when I wanted to give me or other developers a hint for a possible later change. E.g. The "Name" to "2nd Parameter" correspondence could be just by chance or a quick first implementation. What I mean is: maybe "2nd Parameter" has to be localized or mapped later. If you would shorten the code with a direct assignment, I'd assume as a maintenance developer that "Name" always holds a valid "2nd Parameter to IngGenInvPs constructor. And this is maybe not true. This code is not good for other reasons (use switch, use enum, others mentioned it), but not necessarily for the obvious redundance.