public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Detlev Zundel <dzu@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 4/6] sf: Update read/write command macros
Date: Mon, 20 Jan 2014 15:33:01 +0100	[thread overview]
Message-ID: <m238ki910i.fsf@lamuella.denx.de> (raw)
In-Reply-To: <CAD6G_RRVhXoMdezeGu9vocqfiNXwJFNsUfk3Z3DH1E5YEuvoRg@mail.gmail.com> (Jagan Teki's message of "Mon, 20 Jan 2014 17:16:07 +0530")

Hi Jagan,

[...]

> I don't think nothing much gone the readability with these updated:
> CMD_READ_ARRAY_FAST has updated CMD_RD_FAST and it seems like easy to
> understand. and anyway I have added comments for full name as well.

Comments in a far away place cannot compensate for self-explaining
constructs at the location where they are used.  Stating that a
constants needs comments to explain it is actually a good sign that its
name is not chosen carefully enough.

Really, naming constants and variables for readable and maintainable
code is a much harder problem than it looks (cf. my signature) and
unfortunately not easily measurable.  But I assure you that good names
can make a world of a difference.  That's why Marek and me are so
passionate about this seemingly "trivial" change.

> Few of the flashes can be call this as array fast read and fewer call
> this as fast read and few more call this as high frequency
> read. CMD_RD_FAST will suits all these names.
>
> Comments please!

When we change code, we don't do this for the sake of changing it, but
in order to improve one aspect of it, i.e. the performance, the
maintainability or the features.  When everything "stays the same", we
are even _opposed_ to a change because there is nothing to outweigh the
effort to adjusting to the new things.

To summarize - we need proof that a change _improves_ something.
Showing that we "do not loose something" is clearly not enough.

Now in this specific case, we have multiple people voicing the concern
that the renaming looses vital information, thus effectively making
reading and maintining the code harder.  On the other hand even you
agree that "something" although "not much" will be gone after the
rename.  So taking this into account we have only "saving of a few
keystorkes" on the positive side and substantial degradation on
readability and maintainability on the negative side.

Best wishes
  Detlev

-- 
There are two hard things in computer science: cache invalidation,
naming things, and off-by-one errors.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

  parent reply	other threads:[~2014-01-20 14:33 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1390075593-11226-1-git-send-email-jaganna@xilinx.com>
2014-01-18 20:06 ` [U-Boot] [PATCH 1/6] sf: ops: Squash the malloc+memset combo Jagannadha Sutradharudu Teki
2014-01-18 20:34   ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 2/6] sf: Optimize flash features code Jagannadha Sutradharudu Teki
2014-01-18 20:39   ` Marek Vasut
2014-01-18 20:51     ` Jagan Teki
2014-01-20 13:19       ` Marek Vasut
2014-01-20 13:32         ` Jagan Teki
2014-01-20 23:01           ` Marek Vasut
2014-01-21  7:39             ` Jagan Teki
2014-01-21  7:45               ` Jagan Teki
2014-01-21 17:48                 ` Marek Vasut
2014-01-18 20:06 ` [U-Boot] [PATCH 3/6] sf: Renames on dual_flash stuff Jagannadha Sutradharudu Teki
2014-01-18 20:37   ` Marek Vasut
2014-01-18 20:49     ` Jagan Teki
2014-01-20 13:16       ` Marek Vasut
2014-01-20 13:35         ` Jagan Teki
2014-01-18 20:06 ` [U-Boot] [PATCH 4/6] sf: Update read/write command macros Jagannadha Sutradharudu Teki
2014-01-18 20:36   ` Marek Vasut
2014-01-18 20:45     ` Jagan Teki
2014-01-20 11:13       ` Detlev Zundel
2014-01-20 11:46         ` Jagan Teki
2014-01-20 13:06           ` Marek Vasut
2014-01-20 13:10             ` Jagan Teki
2014-01-20 13:13               ` Jagan Teki
2014-01-20 14:33           ` Detlev Zundel [this message]
2014-01-18 20:06 ` [U-Boot] [PATCH 5/6] sf: Minor macro cleanups Jagannadha Sutradharudu Teki
2014-01-18 20:06 ` [U-Boot] [PATCH 6/6] sf: Update bank configuration Jagannadha Sutradharudu Teki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m238ki910i.fsf@lamuella.denx.de \
    --to=dzu@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox