|
Richard Deeming wrote: But it won't work, in the sense that the returned instance will be using the default connection string for the context, not the connection string from the instance passed in.
Hmm. But the parameterless constructor uses the connection string of the (hopefully) already initialized context:
public ModelDataContext() : base(Context.Connection)
However, it's a moot point anyways as I refactored the whole mess into something much less weird.
It'll be an interesting question though to pose to a couple of the devs at work, though I'll reduce the complexity of it omitting the DataContext base class - it's rather superfluous to the example.
|
|
|
|
|
OK, so it's not quite as bad as I though.
You could still break it though:
var context1 = new ModelDataContext(new SqlConnection("server=A;database=B"));
var context2 = new ModelDataContext(new SqlConnection("server=C;database=D"));
CreateNewContext(context1, out var newConn, out var newContext);
Console.WriteLine(newConn.ConnectionString == context1.Connection.ConnectionString);
Console.WriteLine(newContext.Connection.ConnectionString == context1.Connection.ConnectionString);
Console.WriteLine(newContext.Connection.ConnectionString == newConn.ConnectionString);
Console.WriteLine(newContext.Connection == newConn);
Console.WriteLine(newContext.Connection.ConnectionString == context2.Connection.ConnectionString);
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Richard Deeming wrote: You could still break it though:
Yeah, but it's never used that way - two different connection strings. But ideally, it shouldn't be possible to use it that way.
Sigh. The irony of this is that it's code I wrote a while back that my DataContext extension methods rely on, and looking at this now, it's some serious code smell. Fortunately, fixing it affects only a couple web servers that are in operation, but I feel embarrassed. But I blame .NET's DataContext. It does way to much with regards to the state of the Table object. I get what they're trying to do, but there must be a better way that doesn't end up throwing exceptions like "this object was created in a different data context."
|
|
|
|
|
The reason for the results are pretty obvious, when a new instance is created in "CreateNewContext", the parameter "context" is still referring to the original context, not the new one ==> false; When the main program does the compare afterwards, the comparison is done to the static that holds the new context ==> true.
Fixing this is not obvious because you didn't specify what the correct behavior should be.
Do you wish it to be "True" always, or "False" always
Simply update the context parameter in the method to get True always, but I would personally recommend to re-write this whole thing, it is full of code smells. Perhaps some practice in TDD would also be helpful.
Cheers,
Anthony
|
|
|
|
|
It took about a minute to solve.
Assigning over the static for each constructor call is a WTF (as others have called out).
Way too many statics in this Program, if it was rewritten to eliminate all of the statics, then the Program instance would hold a single context and the Main would look like this:
Main(...)
{
new Program().Run();
}
If new instances of the "context" are really needed, then create a Copy constructor to reuse the default context settings on a new instance.
Or use your favorite IOC container.
|
|
|
|
|
It's the assignment of the static member from the instance constructor. HUGE no-no. If you want to make a singleton without using a DI framework, at least make the constructor private and have the static "Context" member be a property that will create a new object if one doesn't exist, and set it to a backing field. Something like this (might need to make Current a method instead of property if you need to pass constructor arguments, but you get the idea):
public class Singleton
{
private Singleton _current;
public Singleton Current
{
get
{
if (_current == null)
{
_current = new Singleton();
}
return _current;
}
}
private Singleton()
{
}
}
|
|
|
|
|
The first 'true' you receive you explicitly cast the object on the function call to a DataContext type.
The second you're comparing a ModelDataContext.Context object - which I'm not sure if it's a DataContext type - if not - then you're comparing unlike objects and the call will fail.
Try explicitly casting the ModelDataContext.Context, and for overkill the newdc to a DataContext type and do the comparison that way.
ie:
Console.WriteLine((DataContext) ModelDataContext.Context == (DataContext) newdc);
|
|
|
|
|
It is a simple newb mistake. Basically, you have written overly complex code making it hard for you to debug. Simplify the syntax as much as possible and the symantec issues will reveal themselves
|
|
|
|
|
...enough to personalise each email just for me and our cat Dij:
Quote: [~forename~}, Treat Your Kitty To The Al La Carte Menu, Minus The Hefty Bill
...
Dear Paul,
...
Typing lessons may help, chaps and chapesses ...
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
AntiTwitter: @DalekDave is now a follower!
|
|
|
|
|
well at least their description of Bill is PC.
This internet thing is amazing! Letting people use it: worst idea ever!
|
|
|
|
|
That's a nice one, [~forename~}!
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
"If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.
|
|
|
|
|
Maybe I should change my username to Original[~forename~} ?
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
AntiTwitter: @DalekDave is now a follower!
|
|
|
|
|
It may break their system
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
"If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.
|
|
|
|
|
So, no downside then?
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
AntiTwitter: @DalekDave is now a follower!
|
|
|
|
|
None; CP will work, regardless of your [~forename~}
Bastard Programmer from Hell
If you can't read my code, try converting it here[^]
"If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.
|
|
|
|
|
Obligatory xkcd: Exploits of a Mom
Freedom is the freedom to say that two plus two make four. If that is granted, all else follows.
-- 6079 Smith W.
|
|
|
|
|
One of my all time favourite XKCD's.
Bad command or file name. Bad, bad command! Sit! Stay! Staaaay...
AntiTwitter: @DalekDave is now a follower!
|
|
|
|
|
OriginalGriff wrote: [~forename~}
Better than {~foreskin~}
Michael Martin
Australia
"I controlled my laughter and simple said "No,I am very busy,so I can't write any code for you". The moment they heard this all the smiling face turned into a sad looking face and one of them farted. So I had to leave the place as soon as possible."
- Mr.Prakash One Fine Saturday. 24/04/2004
|
|
|
|
|
This was inspired by Marc Clifton's post Dumbing down code so it can be maintained by junior devs
I'm a horizontal programmer. I write chainable methods with (short) descriptive names. The top of my methods usually consists of a few "sentences" composed of these "words". If I can put my code on one line, (braces, etc.) I usually do.
Eg, simple method:
string Foo(int x)
{
if(x == 1)
{
return "one";
}
else
{
return "not one";
}
}
}
string Foo(int x){
if (x == 1) {
return "one";
} else {
return "not one";
}
}
string Foo(int x) => (x == 1) ? "one" : "not one";
string Foo(this int x) => (x == 1) ? "one" : "not one";
I do things like this. (Not really this weird though ).
public static List<TSource> WhenNotEmpty<TSource>(this List<TSource> item) => (item.Count > 0) ? item : null;
public static string Prepare(this string item, out string prepared) => ((prepared = item + "Appended") == "HelloAppended") ? "ok" : null ;
public static void Manipulate(this string item, string stuff) => Debug.WriteLine($"{item}:{stuff}");
So I can write in sentences
var list = new List<string> {"a", "Hello", "c"};
list.WhenNotEmpty()?.ForEach(s => s.Prepare(out var r)?.Manipulate(r));
Instead of paragraphs. (something like (methods would be a little different))
var list = new List<string> {"a", "Hello", "c"};
if(list.count > 0){
foreach(var i in list){
var (condition, temp) = Prepare(i);
if(condition != null){
Manipulate(temp);
}
}
}
Getting to the point, (finally )
The paragraphs approach is clearer at the micro level (the details are clear), but the sentences is clearer at the macro level (the overall intent is clear).
Of course you can wrap this paragraph in a method. But when you don't:
Do you tend to write more at the macro/sentence level or micro/paragraph level?
modified 1-Jun-18 8:58am.
|
|
|
|
|
missing a close brace in your first // sometimes me example.
and yeah, sometimes I do the same with similar (obvious) crap.
This internet thing is amazing! Letting people use it: worst idea ever!
|
|
|
|
|
Lopatir wrote: missing a close brace in your first // sometimes me example. Definitely me sometimes.
It was broke, so I fixed it.
|
|
|
|
|
I write C++ in paragraph.
list.WhenNotEmpty()?.ForEach(s => s.Prepare(out var r)?.Manipulate(r));
For me, this is hard to read and maintain.
I'd rather be phishing!
|
|
|
|
|
Maintenance can definitely be harder. Sometimes when I do it this way I have to expand the code to debug and then collapse it back down. I think reading it is a little like shorthand. Once you get used to it, you get a lot of information in a small package.
|
|
|
|
|
I try to write all my code with maintenance in mind, versus trying to get everything on one bloody line.
Just saying...
|
|
|
|
|
Yeah, agree. It has limited use. Mostly I'll do it during clean-up in top-level business methods when I'm trying to communicate complex business logic. I think it is slightly different than some one-liners of low-level code. Each "word" should be closer to a business activity than a data function. The "sentence" should be close to a business process step.
|
|
|
|