Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • World
  • Users
  • Groups
Skins
  • Light
  • Cerulean
  • Cosmo
  • Flatly
  • Journal
  • Litera
  • Lumen
  • Lux
  • Materia
  • Minty
  • Morph
  • Pulse
  • Sandstone
  • Simplex
  • Sketchy
  • Spacelab
  • United
  • Yeti
  • Zephyr
  • Dark
  • Cyborg
  • Darkly
  • Quartz
  • Slate
  • Solar
  • Superhero
  • Vapor

  • Default (No Skin)
  • No Skin
Collapse
Code Project
  1. Home
  2. The Lounge
  3. Refactoring C#, Javascript style.

Refactoring C#, Javascript style.

Scheduled Pinned Locked Moved The Lounge
csharpjavascriptcode-review
34 Posts 14 Posters 0 Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • A Al Beback

    Rob Graham wrote:

    Refactoring to add cleverness?

    Nothing wrong with more cleverness. That's what makes LINQ so appealing.

    Rob Graham wrote:

    I don't see the value here

    The list is not passed into the sub-method each time. That's where I see the value.

    ShamWow

    R Offline
    R Offline
    Rob Graham
    wrote on last edited by
    #22

    Al Beback wrote:

    Nothing wrong with more cleverness. That's what makes LINQ so appealing.

    Almost a guarantee that some maintainer will later screw it up. Cleverness for cleverness sake is bad, particularly when it obscures function or intent.

    Al Beback wrote:

    The list is not passed into the sub-method each time. That's where I see the value.

    No, the list appears not to be passed into the sub-method. You would have to inspect the IL to see if there was a substantial difference in the way the compiler implemented the code, or to determine which was more efficient.

    1 Reply Last reply
    0
    • J J Dunlap

      And if you're using query methods, you might as well use the LINQ syntax. :-)

      public IList GetSomethings()
      {
      var things = new[] { SomeId.Table, SomeId.Chair, SomeId.House, SomeId.Car };

      return (
              from id in things
              where IsAllowed(id)
              select new Something(id)
             )
             .ToList();
      

      }

      A Offline
      A Offline
      Al Beback
      wrote on last edited by
      #23

      J. Dunlap wrote:

      you might as well use the LINQ syntax.

      I don't like LINQ syntax. It's like they shoehorned SQL syntax into C#, unnecessarily. I prefer this a lot more:

      return things.Where(id => IsAllowed(id))
      .Select(id => new Something(id))
      .ToList();

      ShamWow

      J 1 Reply Last reply
      0
      • A Al Beback

        I was going through some C# code I had recently written, when I spotted a cool way to refactor it. See if you agree: BEFORE:

        public IList<Something> GetSomethings()
        {
           var list = new List<Something>();
        
           AddIfAllowed(list, SomeId.Table);
           AddIfAllowed(list, SomeId.Chair);
           AddIfAllowed(list, SomeId.House);
           AddIfAllowed(list, SomeId.Car);
        
           return list;
        }
        
        private void AddIfAllowed(ICollection<Something> list, SomeId id)
        {
           if (IsAllowed(id))
              list.Add(new Something {Id = id});
        }
        

        AFTER:

        public IList<Something> GetSomethings()
        {
           var list = new List<Something>();
        
           Action<SomeId> addIfAllowed = id => 
           {
           	      if (IsAllowed(id))
           	         list.Add(new Something {Id = id});
           };
        
           addIfAllowed(SomeId.Table);
           addIfAllowed(SomeId.Chair);
           addIfAllowed(SomeId.House);
           addIfAllowed(SomeId.Car);
        
           return list;
        }
        

        :cool:

        ShamWow

        S Offline
        S Offline
        Stuart Dootson
        wrote on last edited by
        #24

        I like lambdas and all - but sometimes the language gets in the way.... Haskell-esque:

        GetSomethings = (map Something.filter IsAllowed) [SomeId.Table, SomeId.Chair, SomeId.House, SomeId.Car]

        or, using list comprehensions,

        GetSomethings = [Something x | x <- [1..10], isAllowed x]

        Woo - polymorphism rules! If you want/need a type signature, it'll be something like GetSomethings :: [Id] -> [Something], presuming Something and isAllowed are free. Does C# (or rather, the CLR) have the equivalent of filter and map functions? I know that Linq is the closest to a list comprehension that you'll get in C#...

        Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

        J 1 Reply Last reply
        0
        • J J Dunlap

          And if you're using query methods, you might as well use the LINQ syntax. :-)

          public IList GetSomethings()
          {
          var things = new[] { SomeId.Table, SomeId.Chair, SomeId.House, SomeId.Car };

          return (
                  from id in things
                  where IsAllowed(id)
                  select new Something(id)
                 )
                 .ToList();
          

          }

          0 Offline
          0 Offline
          0x3c0
          wrote on last edited by
          #25

          Not my personal choice. AFAIK, that takes away the on-demand nature of LINQ when it converts to a List. I would personally prefer to return the query straight away - IList<T> inherits from IEnumerable<T> anyway

          Between the idea And the reality Between the motion And the act Falls the Shadow

          J 1 Reply Last reply
          0
          • N Nish Nishant

            If you like that sort of coding style, you might want to use Linq and do something like :

            public IList<Something> GetSomethings()
            {
            return new[] { SomeId.Table, SomeId.Chair, SomeId.House, SomeId.Car }
            .Where(id => IsAllowed(id)).Select(id => new Something(id)).ToList();
            }

            Regards, Nish


            Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
            My latest book : C++/CLI in Action / Amazon.com link

            M Offline
            M Offline
            Mycroft Holmes
            wrote on last edited by
            #26

            The horrifying thing is in a few years you are going to run across this sort of code everywhere and have to support it. Lambda does NOT make your code more readable, I see a support nightmare in the making

            Never underestimate the power of human stupidity RAH

            N 1 Reply Last reply
            0
            • 0 0x3c0

              Not my personal choice. AFAIK, that takes away the on-demand nature of LINQ when it converts to a List. I would personally prefer to return the query straight away - IList<T> inherits from IEnumerable<T> anyway

              Between the idea And the reality Between the motion And the act Falls the Shadow

              J Offline
              J Offline
              J Dunlap
              wrote on last edited by
              #27

              I agree - I was simply following the return type of the original method.

              1 Reply Last reply
              0
              • A Al Beback

                J. Dunlap wrote:

                you might as well use the LINQ syntax.

                I don't like LINQ syntax. It's like they shoehorned SQL syntax into C#, unnecessarily. I prefer this a lot more:

                return things.Where(id => IsAllowed(id))
                .Select(id => new Something(id))
                .ToList();

                ShamWow

                J Offline
                J Offline
                J Dunlap
                wrote on last edited by
                #28

                I will often use the methods directly like that for small queries, but I like the LINQ syntax best for more complex ones. Also, I tend to think they took the good parts of SQL and discarded most of the bad ones (for example they moved the select portion after the from so that intellisense works) - but LINQ vs direct methods is likely just a matter of personal opinion.

                1 Reply Last reply
                0
                • S Stuart Dootson

                  I like lambdas and all - but sometimes the language gets in the way.... Haskell-esque:

                  GetSomethings = (map Something.filter IsAllowed) [SomeId.Table, SomeId.Chair, SomeId.House, SomeId.Car]

                  or, using list comprehensions,

                  GetSomethings = [Something x | x <- [1..10], isAllowed x]

                  Woo - polymorphism rules! If you want/need a type signature, it'll be something like GetSomethings :: [Id] -> [Something], presuming Something and isAllowed are free. Does C# (or rather, the CLR) have the equivalent of filter and map functions? I know that Linq is the closest to a list comprehension that you'll get in C#...

                  Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

                  J Offline
                  J Offline
                  J Dunlap
                  wrote on last edited by
                  #29

                  map= Select() filter= Where() Method as lambda functions:

                  //plain method calls
                  getSomethings= (ts)=> ts.Where(t=>IsAllowed(t)).Select(new Something {Id = id});

                  //LINQ
                  getSomethings= (ts)=> from t in ts where IsAllowed(t) select new Something {Id = id};

                  Unfortunately the getSomethings variable has to be defined and would look like this:

                  Func<IEnumerable<SomeId>,IList<Something>> getSomethings;

                  ...the downside of explicit strong typing.

                  1 Reply Last reply
                  0
                  • J J Dunlap

                    And if you're using query methods, you might as well use the LINQ syntax. :-)

                    public IList GetSomethings()
                    {
                    var things = new[] { SomeId.Table, SomeId.Chair, SomeId.House, SomeId.Car };

                    return (
                            from id in things
                            where IsAllowed(id)
                            select new Something(id)
                           )
                           .ToList();
                    

                    }

                    N Offline
                    N Offline
                    Nish Nishant
                    wrote on last edited by
                    #30

                    J. Dunlap wrote:

                    And if you're using query methods, you might as well use the LINQ syntax.

                    For some reason I can't stand the C# linq syntax - I always use the method calls directly.

                    Regards, Nish


                    Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
                    My latest book : C++/CLI in Action / Amazon.com link

                    1 Reply Last reply
                    0
                    • S Shog9 0

                      I like it, but why cram it all on two lines?

                      N Offline
                      N Offline
                      Nish Nishant
                      wrote on last edited by
                      #31

                      Shog9 wrote:

                      I like it, but why cram it all on two lines?

                      Yeah, normally I'd have new-lined on each of the dots. This was just a quick reply.

                      Regards, Nish


                      Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
                      My latest book : C++/CLI in Action / Amazon.com link

                      1 Reply Last reply
                      0
                      • M Mycroft Holmes

                        The horrifying thing is in a few years you are going to run across this sort of code everywhere and have to support it. Lambda does NOT make your code more readable, I see a support nightmare in the making

                        Never underestimate the power of human stupidity RAH

                        N Offline
                        N Offline
                        Nish Nishant
                        wrote on last edited by
                        #32

                        Mycroft Holmes wrote:

                        Lambda does NOT make your code more readable, I see a support nightmare in the making

                        If we can charge 300/hr fixing that sorta code, I am all for it :rolleyes:

                        Regards, Nish


                        Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
                        My latest book : C++/CLI in Action / Amazon.com link

                        M 1 Reply Last reply
                        0
                        • N Nish Nishant

                          Mycroft Holmes wrote:

                          Lambda does NOT make your code more readable, I see a support nightmare in the making

                          If we can charge 300/hr fixing that sorta code, I am all for it :rolleyes:

                          Regards, Nish


                          Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
                          My latest book : C++/CLI in Action / Amazon.com link

                          M Offline
                          M Offline
                          Mycroft Holmes
                          wrote on last edited by
                          #33

                          If I could charge $300/hr I would fix cobol code.

                          Never underestimate the power of human stupidity RAH

                          N 1 Reply Last reply
                          0
                          • M Mycroft Holmes

                            If I could charge $300/hr I would fix cobol code.

                            Never underestimate the power of human stupidity RAH

                            N Offline
                            N Offline
                            Nish Nishant
                            wrote on last edited by
                            #34

                            Mycroft Holmes wrote:

                            If I could charge $300/hr I would fix cobol code.

                            Yeah me too, in fact I'd port VB6 to Cobol if that! :)

                            Regards, Nish


                            Nish’s thoughts on MFC, C++/CLI and .NET (my blog)
                            My latest book : C++/CLI in Action / Amazon.com link

                            1 Reply Last reply
                            0
                            Reply
                            • Reply as topic
                            Log in to reply
                            • Oldest to Newest
                            • Newest to Oldest
                            • Most Votes


                            • Login

                            • Don't have an account? Register

                            • Login or register to search.
                            • First post
                              Last post
                            0
                            • Categories
                            • Recent
                            • Tags
                            • Popular
                            • World
                            • Users
                            • Groups