** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD ** - Page 47 - Computer Technical Help

Two Plus Two Forums Other Topics Computer and Technical Help

** UnhandledExceptionEventHandler :: OFFICIAL LC / CHATTER THREAD **

Post Reply Subscribe

...

Page 47 of 1603

First

27 37 42 43 44 45 46 47 48 49 50 51 52 57 67 97 147 247 547 1047

Last

Page 47 of 1603

First

27 37 42 43 44 45 46 47 48 49 50 51 52 57 67 97 147 247 547 1047

Last

08-17-2011 , 12:28 AM

#1151

Ankimo

old hand

Join Date: Dec 2010 Posts: 1,996

Quote:

Originally Posted by gaming_mouse

mac users, is it still true that there is no way to maximize a window? and is apple still trying to sell this as a "feature"? or did they finally give in and implement maximize?

Lion lets you fullscreen an app...if that's what you mean.

Quote

$25m Guaranteed WPM on CoinPoker

Join the action now

Daily Rewards • Splash Pots • CoinRaces

08-17-2011 , 07:46 AM

#1152

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Quote:

Originally Posted by gaming_mouse

It's only a problem if the helper functions have problems in them.

I feel like this is taking on the form of a religious debate. You're honestly telling me that if you're going to be modifying or extending code in a class you don't take the time to look at all of the functions in that class? That doesn't pass my common sense test.

Quote:

Originally Posted by gaming_mouse

And anyway if you do the way I suggest, even if you are interested in modifying the implementation code of helper functions for whatever reason, you will find the code faster.

Again, this is your claim. I don't understand at all how you can claim that pressing F3 to take you to your code is faster than the code already being where your cursor is. Maybe it's true for you, but its far from an absolute truth in general.

Quote:

Originally Posted by gaming_mouse

Like I said though, this is silly without a concrete example.

I'll see if I see any today that I can grab and post.

Quote:

Originally Posted by gaming_mouse

Why don't you paste in some function that does multiple things, that you think is better as one big function. I will try to convince that it's better broken down with a specific refactoring.

I'm not advocating for big functions. I'm not saying it's better than a well thought out refactoring. I'm claiming that there are lots of bad refactorings out there and that just because they use small functions doesn't mean they're better than the equivalent big function.

Edit: Bolding the important part that I feel isn't clear between us.

Quote

08-17-2011 , 07:55 AM

#1153

Gullanian

Carpal \'Tunnel

Join Date: Dec 2006 Posts: 14,014

Some bigger continuous pages are preferable in my opinion sometimes (but quite specific/rare). I just wrote an IPN receiver for my website, it's contained, specific, and very easy to debug line by line. I don't mind it being unmodular in this instance but generally of/c favour modular design.

There definitely are people who over modularise things though which ends up obfuscating everything needlessly but it is quite rare to come across from my experience.

Quote

08-17-2011 , 08:11 AM

#1154

ChazDazzle

adept

Join Date: Aug 2005 Posts: 947

Quote:

Originally Posted by Neil S

PHP's haphazard, irregular, randomly constructed library is one of its demerits.

What's great about PHP is that other PHP developers won't chastise you for writing 1000 line functions. It's like a license to ignore OOP. lolwebdeveloperments

Quote

08-17-2011 , 09:02 AM

#1155

gaming_mouse

Carpal \'Tunnel

Join Date: Oct 2004 Posts: 13,786

Quote:

Originally Posted by jjshabado

jj,

as i said the debate is silly at this point without examples. but since you took the time i'll respond to this. you seem to be worried about the programmers who have taken the time to learn about refactoring, but not taken the time to *really* learn, and hence misapply it in their over-zealousness to modularize everything.

i'm not saying this can't be bad, it's just not what i'm generally worried about. i rarely come across it, and when i do it usually has some sense to it, because the authors actually care about the code on some level and are trying to do good even if they suck at it.

the far far more common error is people who have learned to code but never made any effort to learn about standards or refactoring or best practices. they just make all the classic errors that OO patterns and best practices were designed to avoid. duplicate code, lack of encapsulation, tight coupling of unrelated things.

this is the bread and butter of bad code, and as a programmer these are the people whose code will cause you the most pain and whose code you will encounter far more often.

i guess ymmv. but that's my experience.

EDIT: so yeah, i agree with the bolded part in theory. but if you told my one coder wrote average 50 line functions one coder wrote avg 5 line functions, and i had to choose which coder's project to work on without any more info, i'd choose the 5 line guy in a heartbeat.

Quote

08-17-2011 , 09:15 AM

#1156

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Here's a good example from earlier in the thread. We have the code written in 3 ways. The worst of these (to me) is the one that was caused by over zealous refactoring.

It's something that I come across quite a lot in Java (Overzealous following of rules/patterns without thought into why the rule/pattern is being applied).

Like you said though: ymmv.

No refactoring:

Code:

def blowUp(x, y, z):
    if x < y and x < z:
        a,b = y, z
    elif y < z and y < x:
        a,b = z, x
    else:
        a,b = x, y
    print (a * a +  b * b)

Overzealous refactoring:

Code:

def blowUp(x, y, z):
    a, b = andCompare(x, y, z)
    sol = addSquare(a, b)
    print sol

def compare(a, b):
    if a < b:
        return True
    return False

def andCompare(a, b, c):
    if compare(a, b) and compare(a, c):
        return b, c
    elif compare(b, c) and compare(b, a):
        return c, a
    return a, b
    
def square(x):
    return x*x

def addSquare(x, y):
    return square(x) + square(y)

Hybrid:

Code:

def blowUp(x, y, z):
    a,b = dropLowest(x,y,z)
    print (a * a +  b * b)

def dropLowest(a,b,c):
  if a < b and a < c:
    return b,c
  elif b < a and b < c:
    return a,c
  else:
    return a,b

Quote

08-17-2011 , 09:17 AM

#1157

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Quote:

Originally Posted by gaming_mouse

EDIT: so yeah, i agree with the bolded part in theory. but if you told my one coder wrote average 50 line functions one coder wrote avg 5 line functions, and i had to choose which coder's project to work on without any more info, i'd choose the 5 line guy in a heartbeat.

Pure curiosity, around where do you switch?

If I'm told someone wrote average 4 line functions and someone wrote average 25 line functions - I'm probably going with the 25 line dude.

Edit: Or dudette. Not sure if we've had a female post in this forum...

Quote

08-17-2011 , 09:34 AM

#1158

gaming_mouse

Carpal \'Tunnel

Join Date: Oct 2004 Posts: 13,786

Quote:

Originally Posted by jjshabado

yeah i dunno, i probably still go 4 but its closer, irl i'd want to see the actual code and make an informed decision, my hypothetical was just a way of conveying the central point i wanted to make.

Quote

08-17-2011 , 09:37 AM

#1159

Gullanian

Carpal \'Tunnel

Join Date: Dec 2006 Posts: 14,014

It's just silly imo to say 4 line functions > n line functions. In ASP.net this is a totally standard method:

Code:

    /// <summary>
    /// Given a list of category ID's return all specs belonging in them
    /// </summary>
    public static Lite.Spec[] getSpecsForCats(int[] CategoryIDs)
    {
        Lite.Spec[] Specs;

        using(CrystalCommon.MainContext db = new CrystalCommon.MainContext()){
            
            // Get distinct list of all specifications
            var q = (from c in db.tblLiteCategorySpecs
                     join s in db.tblSpecifications on c.SpecID equals s.id
                     where CategoryIDs.Contains(c.CategoryID)
                     select new
                        { s, c })
                    .ToList();
            Specs = new Lite.Spec[q.Count()];
            int i = 0;
            foreach (var Rec in q)
            {
                Specs[i] = new Lite.Spec(Rec.s);
                Specs[i].FriendlyName = Rec.c.FriendlyName;
                Specs[i].CategoryID = Rec.c.CategoryID;
                i++;
            }
        }
       
        return Specs;
    }

I don't see at all how anyone could say it's bad because it's over 4 lines, it's concise and functional, I mean I could probably put it in 4 lines if I were to over modularise it but it would be a massive waste of time and an unmaintainable mess!

Number of lines per method wouldn't factor in at all if I was in recruitment. My favourite Bill Gates quote:

Quote:

Measuring programming progress by lines of code is like measuring aircraft building progress by weight

I think it applies to this debate!

Quote

08-17-2011 , 09:42 AM

#1160

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Not entirely. I mean, I could easily see building the query in a separate function which I think is a pretty big win in terms of readability of that code.

That's probably where I'd stop though (although reasonable people could make an argument for a second 'helper' function that processes the results - making your original function very trivial).

Quote

08-17-2011 , 09:48 AM

#1161

Gullanian

Carpal \'Tunnel

Join Date: Dec 2006 Posts: 14,014

Putting the query in a separate function (assuming we're not talking about stored procedures here) and a helper function to process results would be what I consider unnecessary over modularisation, I think it would be the equivalent of a 1-1 database relationship (99% of the time a 1-1 relationship should just exist in the same table).

Quote:

I mean, I could easily see building the query in a separate function

The way I look at my query, is that it is in a separate function!

Quote

08-17-2011 , 09:59 AM

#1162

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Ok... I don't know ASP.net at all. So I might be doing something ******ed. But I lean towards this level of refactoring.

It seems a bit clearer to me and allows me to easy unit test the non-db part.

I'd also probably (again not sure exactly how things work in ASP.net) move the query to a constant of some sort so that I could easily look over my existing queries in one place.

Code:

    /// <summary>
    /// Given a list of category ID's return all specs belonging in them
    /// </summary>
    public static Lite.Spec[] getSpecsForCats(int[] CategoryIDs)
    {
        Lite.Spec[] Specs;

        using(CrystalCommon.MainContext db = new CrystalCommon.MainContext()){
            
            // Get distinct list of all specifications
            var q = (from c in db.tblLiteCategorySpecs
                     join s in db.tblSpecifications on c.SpecID equals s.id
                     where CategoryIDs.Contains(c.CategoryID)
                     select new
                        { s, c })
                    .ToList();
            Specs = new Lite.Spec[q.Count()];
            int i = 0;
            foreach (var Rec in q)
            {
                Specs[i] = new Lite.Spec(Rec.s);
                Specs[i].FriendlyName = Rec.c.FriendlyName;
                Specs[i].CategoryID = Rec.c.CategoryID;
                i++;
            }
        }
       
        return Specs;
    }

Code:

    /// <summary>
    /// Given a list of category ID's return all specs belonging in them
    /// </summary>
    public static Lite.Spec[] getSpecsForCats(int[] CategoryIDs)
    {
            var q = getDbSpecs(categoryIDs);
            return buildSpecsFromDbQuery(q);
    }

    private static List getDbSpecs(int[] CategoryIDs) {
         using(CrystalCommon.MainContext db = new CrystalCommon.MainContext()) {
            
            // Get distinct list of all specifications
            return (from c in db.tblLiteCategorySpecs
                     join s in db.tblSpecifications on c.SpecID equals s.id
                     where CategoryIDs.Contains(c.CategoryID)
                     select new
                        { s, c })
                    .ToList();
         }
      }

      private static List.Spec[] buildSpecsFromDbQuery(var q) {
            Specs = new Lite.Spec[q.Count()];
            int i = 0;
            foreach (var Rec in q)
            {
                Specs[i] = new Lite.Spec(Rec.s);
                Specs[i].FriendlyName = Rec.c.FriendlyName;
                Specs[i].CategoryID = Rec.c.CategoryID;
                i++;
            }
            return Specs
    }

Edit: Crazy* people would then try to separate out constructing a single record, and break that up into individual functions for setting properties.

* Crazy obviously means someone that doesn't 100% agree with me.

Quote

08-17-2011 , 10:04 AM

#1163

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Quote:

Originally Posted by Gullanian

But there are lots of reasons to break a function up. I've given two above:
1) I want to test functionality separate from actual database logic
2) I want to have my DB queries in one place (or at least by themselves) so I (or a DBA) can look over them easily.

My problem is when people can't give me a reason for breaking up a function besides "It makes it smaller".

Quote

08-17-2011 , 10:04 AM

#1164

gaming_mouse

Carpal \'Tunnel

Join Date: Oct 2004 Posts: 13,786

yes obv i agree that example is bad. EDIT: meaning the lisp example from dave, i still have to read the other ones

Quote

08-17-2011 , 10:07 AM

#1165

Gullanian

Carpal \'Tunnel

Join Date: Dec 2006 Posts: 14,014

I get what you are saying, maybe this is just difference of opinion (but I strongly suspect not), but because I consider those functions to have a one-to-one relationship with each it is totally unnecessary. You can often pre-empt segments of functions to modularise, but this doesn't mean you need to pre-empt everything, a lot of functions like the one I provided function just great on their own and can be modularised when the time comes to do so, not before.

You say it would benefit readability, but I honestly think that the original is a lot more readable. Which makes me think perhaps this is a style/preference issue but I don't think it is if I'm going to be honest about it!

Quote

08-17-2011 , 10:08 AM

#1166

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Quote:

Originally Posted by gaming_mouse

yes obv i agree that example is bad. EDIT: meaning the lisp example from dave, i still have to read the other ones

But that was literally the level of refactoring I was encountering when I went on my rant!

To Dave: See, your little learning exercise ended up with code just as good as what a lot of 'professional' Open Source contributors do. So you're not doing too badly.

Quote

08-17-2011 , 10:10 AM

#1167

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Quote:

Originally Posted by Gullanian

You say it would benefit readability, but I honestly think that the original is a lot more readable. Which makes me think perhaps this is a style/preference issue but I don't think it is if I'm going to be honest about it!

I do think the refactored one is a bit more readable - but I totally accept that that is a personal preference. In the end I went with the two reasons I listed (which I think are valid) for why I'd want to refactor the function.

Quote

08-17-2011 , 10:13 AM

#1168

Gullanian

Carpal \'Tunnel

Join Date: Dec 2006 Posts: 14,014

In timed tests to see how long it takes a group of programmers to tell me what each example does, the original I provided would win versus the 3 smaller functions by a significant margin imo. I think that's what readability is at the end of the day although definitions may vary.

Quote

08-17-2011 , 10:16 AM

#1169

jjshabado

Carpal Tunnel

Join Date: Jul 2006 Posts: 22,732

Gullanian:

1) Do you agree or not with my two reasons for refactoring the function?
2) What answer are you looking for? I mean isn't the answer just:

Code:

/// <summary>
/// Given a list of category ID's return all specs belonging in them
/// </summary>

Quote

08-17-2011 , 10:31 AM

#1170

gaming_mouse

Carpal \'Tunnel

Join Date: Oct 2004 Posts: 13,786

Quote:

Originally Posted by jjshabado

This. jj, I think in practice we're gonna be on the exact same page about most things.

Quote

08-17-2011 , 10:34 AM

#1171

gaming_mouse

Carpal \'Tunnel

Join Date: Oct 2004 Posts: 13,786

Quote:

Originally Posted by Gullanian

You say it would benefit readability, but I honestly think that the original is a lot more readable.

wrong.

Quote:

a style/preference issue but I don't think it is if I'm going to be honest about it!

absolutely correct!

Quote

08-17-2011 , 10:34 AM

#1172

tyler_cracker

Carpal \'Tunnel

Join Date: Apr 2005 Posts: 15,828

Quote:

Originally Posted by Gullanian

shabby has hit this but for me the punch line is: show me your unit tests, and i'll tell you how well your code is architected.

fwiw i generally agree with mouse's position: the magnitude of failure is much higher in code that is too big vs too small, the frequency of failure skews toward too big rather than too small, hence big code is "worse" than small code (has larger -EV).

shabby, i appreciate your side but dredging up an example that began as a hw problem rather than a real-world problem illustrates exactly how hard one has to try to produce code that is "too refactored". let's see some real world code, mang!

Quote

08-17-2011 , 10:38 AM

#1173

Gullanian

Carpal \'Tunnel

Join Date: Dec 2006 Posts: 14,014

The question I would ask would be "tell me how it works" I suppose.

For an application where you want to separate DB logic from code logic further I would favour a stored procedure over modularising every query in code. I suppose for your points I do agree in principal but I think it depends largely on the project.

Quote

08-17-2011 , 10:41 AM

#1174

Gullanian

Carpal \'Tunnel

Join Date: Dec 2006 Posts: 14,014

Quote:

Originally Posted by gaming_mouse

wrong.

I mean, when we are talking about something subjective like readability, is it not perhaps a bit arrogant to declare my opinion is flat out wrong?

Perhaps you could start by defining readability, because I think being able to explain what a piece of code does in the fastest time possible is a pretty strong definition.

Quote

08-17-2011 , 10:49 AM

#1175

gaming_mouse

Carpal \'Tunnel

Join Date: Oct 2004 Posts: 13,786

Quote:

Originally Posted by Gullanian

it is, i was being deliberately provocative, though I do think it is a pretty clear cut case of better and worse. and by the definition you just gave, jj's refactoring wins imo. The main reason is that it gives you immediate access to a high-level description of the code, which gets lost in implementation details in your version. So what does the code do?

Code:

var q = getDbSpecs(categoryIDs);
            return buildSpecsFromDbQuery(q);

"Oh, it grabs specs based on category ids from the db, then it builds those specs based on the results"

Now if you want to know more detail about one of those things, you can drill down. But the key point is exposing a high-level summary of the algorithm, and then allowing the reader of the code to easily get more detail about one specific piece of it if he wants. The high-level summary is what's missing from the original code.

EDIT: I would add that in your case the "error" is small. That is the original code is still readable, it only does 2 things, it's totally fine. But the refactored version is an improvement.