|
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.
|
|
|
|
|
+5
|
|
|
|
|
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
|
|
|
|
|
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.
modified 3-Jan-12 8:51am.
|
|
|
|
|
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
|
|
|
|
|
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.
|
|
|
|
|
"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.
|
|
|
|
|
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
|
|
|
|
|
Switch statements in Java didn't support strings as arguments until recently. Objective C still doesn't. So no training required 
"You get that on the big jobs."
|
|
|
|
|
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
|
|
|
|
|
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!!!"
|
|
|
|
|
He forgot to wrap each line in a try catch block! o_O
|
|
|
|
|
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.
|
|
|
|
|
Today while reviewing one of the code, i found this chunk 
class MyClass
{
private int m_SomeValue;
public int SomeValue
{
get { return m_SomeValue;}
}
public void SomeMethod()
{
var value = GetValue();
}
private int GetValue()
{
return SomeValue;
}
}
At first i started to think is there any catch behind this style. after few mins, i found its lame to write this way and finally it was admitted 
|
|
|
|
|
I think the code is attempting to do something like this:
public int SomeValue {get; private set;}
|
|
|
|
|
Well nope, but still there wasnt a method needed to access/retrieve a instance member value
|
|
|
|
|
It's called "double encapsulation"! Wrap your field in a property, then wrap the property in a method! It will be even more object-oriented that way!
|
|
|
|
|
lol i bet it is!!
|
|
|
|
|
And don't forget to declare both the field and the property as private. Otherwise other bad programmers might accidentally access them.
|
|
|
|
|
Bit hacks[^]
For example: Round up to the next highest power of 2:
unsigned int v;
v--;
v |= v >> 1;
v |= v >> 2;
v |= v >> 4;
v |= v >> 8;
v |= v >> 16;
v++;
This makes my previous code soooo lame (Math.Pow(2,Math.Log(v,2)+1)) or sth like this).
Greetings - Jacek
|
|
|
|
|
|
|
I recently started working at a large corporation that transitioned from a waterfall to an agile approach to software development about 2 years ago. As a part of the transition, developers who never fully understood test-driven development decided to start writing unit tests for their code to make it feel more agile.
The results of their efforts are hundreds of unit tests with no clear purpose, goal, naming conventions, or coding standards. While trying to wrap my head around what possible benefit the unit tests offer, the best thing I can say about them is that they prove that random chunks of production code in private and public methods do the same thing as the unit tests which duplicate their logic whenever the constants specified in the unit tests match the values that haven't changed in the DEV database that they never mocked and the database connections and web services don't time out on them.
I guess, in some abstract sense, running these unit tests could provide some benefit to the system, but we could improve the benefits by ignoring the unit tests and hiring a witch doctor to stand next to the source control server, sense the code's ethereal energy, and tell us if the code feels cursed or not.
Know any witch doctors looking for employment?
|
|
|
|
|
Perhaps you could set the server on a nice, comfortable couch and have a psychologist discuss the "daddy issues" with the code, due to inept "parent" programmers..?
Same thing.
|
|
|
|
|
jesarg wrote: but we could improve the benefits by ignoring the unit tests and hiring a witch doctor to stand next to the source control server, sense the code's ethereal energy, and tell us if the code feels cursed or not.
Just along for the ride.
"the meat from that butcher is just the dogs danglies, absolutely amazing cuts of beef." - DaveAuld (2011)
"No, that is just the earthly manifestation of the Great God Retardon." - Nagy Vilmos (2011)
"It is the celestial scrotum of good luck!" - Nagy Vilmos (2011)
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
