|
Adding and removing COLUMNS in a dataset while attempting to SAVE? Dear God, is he running ALTER TABLE statements in a save routine? Or just removing columns he doesn't actually want to persist?
|
|
|
|
|
Perhaps that's perfectly normal on some obscure planet where he comes from.
It rally truned out that he casually binds the dataset to a report before saving, so that you can print that out. The columns are added or removed according to the user's role and are perhaps added again after opening the report. Ahh, yes, he had to fill the some values into the newly added columns. He only does that in the first rows of the tables and ignores all others. That's also a common pattern in one of his other creations. He always assumes that datatables are filled with exactly one row, no more, no less. Anyway, he must have discovered that he needed an XML schema of the dataset, so he simply saves it after finishing his manipulation. Great idea. This way the schema is always accurate, no matter which route we took through the spaghetti code.
My best guess is that the report was added later and that he needed two or three slightly different reports, depending on the user's role. Anybody exept him would have clicked together a schema, created a typed dataset from it and added different queries to fill it for the different roles. Filling and binding it would have been something around 15 lines of code.
But don't worry too much about the database. Not a single table had a primary key and of course no foreign keys or other constraints. Why should he need them since he apparently believes that his queries always return exactly one row? I would not put it beyond him to simply reverse his manipulations in the dataset before saving. I honestly will not look if I don't have to.
I'm invincible, I can't be vinced
|
|
|
|
|
Behold!
I found something like this:
protected void Page_Load(object sender, EventArgs e)
{
List<Node> Stuff = loadStuff();
}
private List<Node> loadStuff()
{
List<Node> Stuff = new List<Node>();
someMoreStuff:
foreach (Node thing in Node.GetCurrent().Children)
{
Stuff.Add(thing);
}
Stuff = ShuffleNodeList(Stuff);
if (Stuff.Count < 100 && Stuff.Count > 0)
{
goto someMoreStuff;
}
return Stuff;
}
It worked... but I changed it so that it never happened:
protected void Page_Load(object sender, EventArgs e)
{
var Stuff = new List<Node>();
Stuff = loadStuff(Stuff);
}
private List<Node> loadStuff(List<Node> Stuff)
{
foreach (Node thing in Node.GetCurrent().Children)
{
Stuff.Add(thing);
}
Stuff = ShuffleNodeList(Stuff);
if (blablabla)
{
loadStuff(Stuff);
}
return Stuff;
}
Edit: So I changed it again...
protected void Page_Load(object sender, EventArgs e)
{
List<Node> Stuff = loadStuff();
}
private List<Node> loadStuff()
{
List<Node> Stuff = new List<Node>();
do {
foreach (Node thing in Node.GetCurrent().Children)
{
Stuff.Add(thing);
}
} while (Stuff.Count < 100 && Stuff.Count > 0))
return ShuffleNodeList(Stuff);
}
Giraffes are not real.
modified 21-Feb-12 11:13am.
|
|
|
|
|
Does C# do tail recursion natively? If not, there is an argument that the original is better, though it should be commented as such.
|
|
|
|
|
No idea... I assumed it was worse. Seen code of my senior colleagues using tail recursion too. (Monkey see, monkey do.)
Turns out it's not supported in C#; only in the CLR.
http://lookingsharp.wordpress.com/2010/03/08/tail-recursion-in-csharp-and-fsharp/[^]
The workaround looks interesting, but way too hardcore for the things the module is intended for. I will rarely need to loop, it's just to make sure it's flexible in case someone starts messing with the table while the site is live.
I wrote the first piece of code because I couldn't get it to work otherwise. It's pretty funny if it turns out to be better practice.
Giraffes are not real.
|
|
|
|
|
WHYYYYY??? If you're declaring Stuff in Page_Load and never doing anything with it, just nuke it all! Or did you not show the code that actually uses Stuff to populate the page or whatever?
|
|
|
|
|
Yes it's used for something of course. I'm not 'that' stupid.
Edit: and I'm not actually calling my variables "Stuff" either in case you're wondering.
Giraffes are not real.
|
|
|
|
|
0bx wrote: and I'm not actually calling my variables "Stuff" either
No? I'm disappointed. I found that rather original.
|
|
|
|
|
They're both horrors.
You don't ever need to return Stuff. Not to mention this "ShuffleNodeList" function is called each time loadStuff is called (I'm guessing it should only be called after everything has been loaded).
I wonder... is this code that is using the Umbraco API? Some of that looks familiar.
|
|
|
|
|
Yes it's the Umbraco api and it's not the complete code, it returns the list to be used in an other function that generates html in order to make the page look different every time you visit it.
Having the ShuffleNodeList was at the end at first... I though I had a good reason to put it in between at first, but looking at it doesn't make sense anymore.
Giraffes are not real.
|
|
|
|
|
0bx wrote: it returns the list to be used in an other function
But the calling function already has the list, as it was passed in as a parameter, so it does not need to be returned since it already exists at the location it would be returned to. I suppose returning it could make for shorter syntax, but not in the case you have shown.
|
|
|
|
|
I wonder, why u had to use recursion/goto at all:
protected void Page_Load(object sender, EventArgs e)
{
var Stuff = loadStuff();
}
private List<Node> loadStuff()
{
var Stuff = new List<Node>();
do
{
foreach (Node thing in Node.GetCurrent().Children)
{
Stuff.Add(thing);
}
Stuff = ShuffleNodeList(Stuff);
}
while (blablabla);
return Stuff;
}
|
|
|
|
|
On the no need for recursion and getting rid of the goto, can I add only doing a single shuffle:
protected void Page_Load(object sender, EventArgs e)
{
List<Node> Stuff = loadStuff();
}
private List<Node> loadStuff()
{
List<Node> Stuff = new List<Node>();
do {
foreach (Node thing in Node.GetCurrent().Children)
{
Stuff.Add(thing);
}
} while (Stuff.Count < 100 && Stuff.Count > 0))
return ShuffleNodeList(Stuff);
}
Nice, self contained and easy to follow.
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
|
|
|
|
|
Okay, I knew about the "While" loop, but I didn't know you can do it like this.
Giraffes are not real.
|
|
|
|
|
Chuckle! I think a lot of us do the same, when the occasion arises. I certainly do. Of course, there's no stigma involved in the practice, as long as you remember to test your replacement code thoroughly. Right? Right?
(As it happens, I also do it to my old, already-published fiction...which causes my readers a bit of concern when they discover it! Well, at least with fiction the "debugging" is less onerous.)
|
|
|
|
|
Gah! You needed that goto for the "Goto Hell" Achievement!
I remember when goto and gosub/return were the ONLY way to get around in your program flow, how times have changed!
|
|
|
|
|
|
0bx wrote: foreach (Node thing in Node.GetCurrent().Children)
{
Stuff.Add(thing);
}
Others have commented on the outer looping constructs. Regarding the inner foreach loop, I'd just get rid of it:
Stuff.AddRange(Node.GetCurrent().Children);
Cheers.
|
|
|
|
|
What if Stuff.Count never exceeds 100 items? Both versions seem like infinite loops.
Recursion would be superior here because it will at least blow the call stack in a reasonable time frame vs. waiting for the application container (IIS?) to kill the non-responsive thread.
I usually include a safety valve for that kind of code:
int maxReps = 100;
blalbalba && (maxReps-- > 0)
I always use safety valves when writing code that "should" converge to an answer.
|
|
|
|
|
How about?:
protected void Page_Load(object sender, EventArgs e)
{
List<Node> Stuff = new List<Node>();
do {
foreach (Node thing in Node.GetCurrent().Children)
Stuff.Add(thing);
} while (Stuff.Count < 100 && Stuff.Count > 0))
Stuff = ShuffleNodeList(Stuff);
}
|
|
|
|
|
Journalists of Bangladesh Asked to police "when are you going to catch the criminal."
Police ob Bangladesh answered that when they will be 100% confirm.
Then the journalist asked "What do you mean by 100% confirm"
Police department answered:
When someone will give the detail description how they murdered those Journalist couple in-front of Journalists then they will be 100% confirm.
FÚck, I born this shîtty country
modified 16-Feb-12 1:17am.
|
|
|
|
|
|
its a programming forum dude,
even though its a shame
|
|
|
|
|
This is the original code:
DateTime dtInput;
DateTime.TryParse(txtInputDate.Text, out dtInput);
document.InputDate = dtInput;
Lets do a change request:
* Input date always will be the current date.
DateTime dtInput;
DateTime.TryParse(DateTime.Now.ToString(), out dtInput);
document.InputDate = dtInput;
What is wrong with these people? They start coding and stop thinking?
|
|
|
|
|
I think the best part of this one is that even the original code is broken – because it's not checking the result of TryParse and assigning a sensible default value, if the input string wasn't in a valid format, you get (DateTime)0 which is almost certainly not what they want (and will cause things further down the line to break in subtle ways).
|
|
|
|