|
Aside from the obvious, you might also want to do some reviews for SQL injection vulnerabilities.
|
|
|
|
|
pretty much sums it up, here too.
Andrew Rissing wrote: do some reviews for SQL injection vulnerabilities
Yes, they should. They ought to do a search on this site for Colin Mackay's article on such topic
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
http://xacc.files.wordpress.com/2008/10/helpme.png[^]
I think I need to give the office a crash course in R E F A C T O R I N G.
Note: The #2 version is simply a copy/paste of the first with a bit of extra functionality. Honestly, I would not let anyone even see the code inside.
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
Nice. Anyway I strongly doubt the developer who created such brilliant method names will agree about the need of refactoring.
Well, I forgot you're the boss!
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
CPallini wrote: Well, I forgot you're the boss!
The author of the code is my boss (the IT Director) ...
xacc.ide - now with TabsToSpaces support IronScheme - 1.0 beta 1 - out now! ((lambda (x) `((lambda (x) ,x) ',x)) '`((lambda (x) ,x) ',x))
|
|
|
|
|
leppie wrote: The author of the code is my boss (the IT Director) ...
Oh, don't worry, I think his/her skills are well above the average ones of the IT directors I ever had.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
As a former boss of mine once told me: "People will be promoted to their levels of incompetence"
And as I've seen firsthand, if management likes someone who is a dumb as a stump, they will be moved into a position where they can do the least damage.
I don't claim to be a know it all, for I know that I am not...
I usually have an answer though.
|
|
|
|
|
..and there you have the recruitmentprocedure for management-positions in the banking-world
|
|
|
|
|
lol, have fun
|
|
|
|
|
Are you his boss?
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
Verb/Noun pair -- check! So what's the problem?
|
|
|
|
|
PIEBALDconsult wrote: Verb/Noun pair -- check! So what's the problem?
And the copy paste is just good re-use of code, right?
Bill W
Just because code works, it doesn't mean that it is good code.
|
|
|
|
|
And orthogonal too.
|
|
|
|
|
Am I seeing right?
Does one of these 'functions' really have around 1500 lines ???
I normally consider more than 20/25 LOC bad (except it is a trivial switch statement or something of that kind)...
And even worse, the 'programmer' who created this nightmare is your boss??
Poor you, get out of there as quick as possible (or at least prevent your boss from coding somehow)!
Regards
Thomas
|
|
|
|
|
Seriously, 1467 lines worth of copy and pasting.
I don't care if he/she's your boss, shoot them, or better yet, give me their id, I'll do it...
|
|
|
|
|
In a new body of code I've "inherited" there is a lot of code that looks something like this (simplified somewhat):
public class A
{
private ArrayList list = new ArrayList();
private object m_lock = new object();
public A()
{
lock(m_lock)
{
list.Add("MyString");
}
}
}
Now am I going mad? That lock statement to my eye is marginally pointless. Is there ever any justification for locking on a non static, private object in a constructor?
Cheers,
E.
|
|
|
|
|
Probably not, and why have a separate object for the lock either (in the included code anyway)?
But it could be worse as well.
|
|
|
|
|
[I'll get there, just give me a sec. ]
From a coding perspective, people often use 'this' as the object to lock on, but that is not a preferred method of locking. The reason is that code outside of the class might want to use that object to lock on as well, there by possibly causing a deadlock. So, the recommendation is to often use internal private objects to do this.
Now, in this case, I would assume the array list would be the best item to lock on. But out of readability or perhaps if that array list is reinitialized later, they may have opted for a separate unchanging object reference for the lock to prevent any kind of concurrency issues.
So, in short, the lock in the constructor is needless, but the object used in the lock very well could be needed depending on the implementation of the class.
|
|
|
|
|
The previous position of the developer was in jeweller’s shop.
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
|
|
I was going through our documentation and noticed this gem. We had a senior developer who created our development standards. This was part of the document he wrote for us. We had the .NET 2.0 framework available to us (hint: Generics) when this was written. I hope you find this as entertaining as I did.
On a side note, when the efficiencies were brought to his attention and refuted with a sample app, he was indignant and would not even consider any other way. He no longer works here!
There are times that information needs to be collected and distributed around the program. To aid in the collection of data and
passing it, Microsoft added collections. The two most popular are arrays and arraylists.
An array takes its space out of the near heap. It is very possible to run this out by adding to the array. Arraylist takes its
space out of the far heap and in 64KB chunks. When building the collection, arraylists are the best thing to use for speed.
HOWEVER, when passing the information from method to method, an arraylist is not the most efficient taking up to 60,000 flops to pass
the information therefore arrays are faster.
q: How do we strike a happy medium on this?
a: Use both. Since most of the data that we collect are strings, we can do the following. Build the arraylist when collecting
data, the pass the data to an array when collecting is complete. The following code is a great example on this:
string[] results = new string[arraylist.count];
arraylist.CopyTo(results, 0);
return results;
|
|
|
|
|
I believe he didn't learn any programming languages coverd Array before he jumped into Senior position of .NET
|
|
|
|
|
Yep. He often regaled us about the early days of his TRS-80 and the "Line Numbered BASIC" programs he wrote. I seriously wonder if he still fires up that machine at night...
Hogan
|
|
|
|
|
Why dont you write your own IList object like to your queries
|
|
|
|