Кирилл Корняков, Андрей Морозов
Сентябрь 2018
Наши клиенты не смотрят на исходный код.
Почему мы должны держать его в чистоте?
Генеральный директор компании по разработке ПО,
Нижний Новгород, Август 2010
Хорошо |
Плохо |
|
|
Кто виноват?
Say what you mean, simply and directly
Write clearly — don't be too clever
Write clearly — don't sacrifice clarity for "efficiency"
The Elements of Programming Style (1978)
You know you're brilliant, but maybe you'd like to understand what you did 2 weeks from now. If you need more than 3 levels of indentation, you're screwed anyway, and should fix your program.
Matrix createMatrix(int a1, int a2); // Bad
Matrix createMatrix(int width, int height); // OK
List<PECustomerDetailsData> retrieveValidateAndConvertCustomerSpecificDataIntoPresentationEntities(); // Bad
customer.customerName = "Bill";
public void do(); // move up
private int i1; // move down
// Bad
public List<int[]> getThem() {
List<int[]> list1 = new ArrayList<int[]>();
for (int[] x: theList) {
if (x[0] == 4)
list1.add(x);
}
return list1;
}
// OK
public List<Cell> getFlaggedCells() {
List<Cell> flaggedCells = new ArrayList<Cell>();
for (Cell cell: gameBoard) {
if (cell.isFlagged()) {
flaggedCells.add(cell);
}
}
return flaggedCells;
}
int dailyPay = hourlyRate * 8;
double milesWalked = feetWalked / 5280;
int step = width * 4;
Не используйте их!
Вместо этого используйте:
WORK_HOURS_PER_DAY
FEET_PER_MILE
sizeof(int)
public class Part {
private String m_dsc;
void setName(String name) {
m_dsc = name;
}
}
public class Part {
private String description;
void setDescription(String description) {
this.description = description;
}
}
Имена классов и объектов должны представлять собой
существительные и их комбинации.
Хорошо
Customer
WikiPage
Account
Плохо
Manager
-> AccountManager
Processor
-> JobProcessor
Data
-> Order
Info
-> HelpMessage
Имена методов представляют собой
глаголы или глагольные словосочетания.
String name = employee.getName();
customer.setName("Mike");
if (paycheck.isPosted()) ...
Хорошо
public class IncompleteOrder {}
public int currentPosition = -1;
const int WORK_HOURS_PER_DAY = 8;
private bool isBlocked // can, is, has
Плохо
public class incompleteOrder {}
private bool flag;
public const int NUMBEROFCONTEXTS = 10;
private int collectionsize;
private string m_strName;
private byte _array;
Какова нормальная длина функции?
In a completely uncommented 2000 line method:
{
{
while (..) {
if (..){
}
... (just putting in the control flow here, imagine another few hundred ifs)
if(..) {
if(..) {
if(..) {
...
(another few hundred brackets)
}
}
} //endif
...
endif
showed up around line 800! int OverlayFlatVideos(int numberOfFlatVideos,
int currentFrameIdx,
OverlayAllVideosParams^ previewParams,
std::vector<bool>& StreamProcessed,
std::vector<acvCapture*>& flatVideoReaders,
std::vector<double>& fpsFlatVideos,
bool sharedReflection,
std::vector<CvMat*>& reflectionsFlatToDome,
CvSize& fullDomeSize,
std::vector<IplImage*>& masks,
std::vector<IplImage*>& borderSmoothImage,
CvSize& maskSize,
IplImage*& imageFullDome,
CvMat*& tempRef,
double fps,
int& numberOfVideoReaders,
IplImage*& imageReflected,
IplImage*& imageFullDomeCopy,
InterpolationMethod inMethod) // 19
static void GetSupportDocFilePath(out string supportDocFilePath) {
supportDocFilePath = new ConfigurationHelper().SupportFilePath;
}
GetSupportDocFilePath(path);
Или лучше так?
static string GetSupportDocFilePath() {
return new ConfigurationHelper().SupportFilePath;
}
path = GetSupportDocFilePath();
float _______ ( float number )
{
long i;
float x2, y;
const float threehalfs = 1.5F;
x2 = number * 0.5F;
y = number;
i = * ( long * ) &y; // evil floating point bit level hacking
i = 0x5f3759df - ( i >> 1 ); // what the f*ck?
y = * ( float * ) &i;
y = y * ( threehalfs - ( x2 * y * y ) ); // 1st iteration
// y = y * ( threehalfs - ( x2 * y * y ) ); // 2nd iteration, this can be removed
return y;
}
if (splitParameters->projectorVideos == nullptr ||
System::String::IsNullOrEmpty(splitParameters->splitSettings) ||
splitParameters->projectorWidth <= 0 ||
splitParameters->projectorHeight <= 0)
if (timer.HasExpired() && !timer.IsRecurrent()) // Bad
if (ShouldBeDeleted(timer)) // OK
if(isValid == false) // Bad
if(!canEditPrice) // OK
public void SendShutDown() {
var handle = GetHandle(device);
if (handle != DeviceHandle.INVALID) {
var err = OpenDevice(handle);
if (err == NULL) {
//
}
else {
Logger.Log("Can't open device. Error: " + err.ToSting());
}
}
else {
Logger.Log("Invalid handle for: " +
device.ToSting());
}
}
public void TrySendShutDown() {
try {
SendShutDown();
}
catch() {
// ...
}
}
public void SendShutDown() {
var handle = GetHandle(device);
OpenDevice(handle);
//...
}
"Don’t comment bad code — rewrite it!"
B. Kernighan, P. Plauger "The Elements of Programming Style"
"...the only software documentation that actually seems to satisfy the criteria of an engineering design is the source code listings".
Jack Reeves
# Комментарии — это плохо!
// When I wrote this, only God and I understood what I was doing
// Now, God only knows
...
// Magic. Do not touch.
...
// sometimes I believe compiler ignores all my comments
...
// I'm sorry.
...
// I am not sure if we need this, but too scared to delete.
catch (Exception e) {
//who cares?
}
Они полезны?
/*---------------------------------------------------------------
-----------------------
Created by: NANDA
Created Date: 01-AUG-2009
Modified by:
Procedure Description: Fetches menu items based on the given
user permission
Input Parameters: LoginEntry user
Output Parameters: none
----------------------------------------------------------------
--------------------*/
public BEMenuList FetchMenuItems(LoginEntity user) {
...
}
...
// Gets the login user id
// Gets the CRM details
FetchCrmDetails();
...
// If the server variable is empty, throw the error message
if (loginUserId == null)
{
throw new Exception("No User Id");
}
public void LoadProperties() {
try
{
var propertiesPath = propertiesLocation +
"/" + PROPERTIES_FILE;
var propertiesStream = File.Open(propertiesPath);
loadedProperties.Load(propertiesStream);
}
catch (IOException ex) {
//If file with properties doesn’t exist,
//default settings are loaded
}
}
//#region ShowHyperLink
///// <summary>
///// Show the hyperlink controls
///// </summary>
//private void ShowHyperLink()
//{
// asphypCreateAccounts.Visible = true;
// asphypCreateAccounts.NavigateUrl = "CreateAccount.aspx";
// asphypCreateAccounts.Text = WebConstants.CreateAccountHyperLink;
// asphypCreateExtUser.Visible = true;
// asphypCreateExtUser.NavigateUrl = "ManageExternalUsers.aspx";
// asphypCreateExtUser.Text = WebConstants.ExternalUserHyperLink;
//}
//#endregion
asplblAcceptDeclineDate.Text = contractHistoryList[0].AcceptedDate.ToString();
if (contractHistoryList[0].IsMultiple)
{
asplblMultiplePublish.Text = "Yes";
}
else
{
asplblMultiplePublish.Text = "No";
}
//if (contractHistoryList[0].IsLegalApprovalRequierd == true)
//{
// asplblLegalApproval.Text = "Yes";
//}
//else
//{
// asplblLegalApproval.Text = "No";
//}
asphdnFileGuid.Value = contractHistoryList[0].FileGuid.ToString();
//int contractRefNo = Convert.ToInt32(asphdnFileMasterld.Value.ToString());
//Session(WebConstants.CONTRACT_REF_NO_SESSION_KEY] = contractRefNo;
asptxtReasonForRejection.Text = string.Empty;
/**
* Always returns true.
*/
public boolean isAvailable()
{
return false;
}
// format matched kk:mm:ss EEE, MMM dd, yyyy
Pattern timeMatcher = Pattern.Compile("\\d*:\\d*:\\d* \\w*, \\w* \\d*, \\d*");
//TODO: ...
//FIXME: ...
//HACK, NOTE, WARNING
Плохо
customer.CalculateCredit ( fromDate );
customer.CalculateCredit(fromDate , toDate);
if ( IsValid ) i ++
Хорошо
if(customer.IsValid && customer.Credit == 0.0)
position = new Location(position.x + 10, position.y);
return IsValid ? cdd : DateTime.MaxDate;
using MVCS.Diff4Big.Domain.ImageEntities;
using MVCS.Diff4Big.Domain.Specifications;
namespace MVCS.Diff4Big.Domain.Comparison.FT {
public class ByteByByte : ITileComparator {
private readonly LengthBased lengthBased = new LengthBased();
public IDeltaContainer Compare(IContentTile tile1, IContentTile tile2, ChangeType changeType) {
var bytesRangeEqualsSpecification = new BytesRangeEquals(tile1.Content);
var baseResult = lengthBased.Compare(tile1, tile2, changeType);
if (baseResult != null) return baseResult;
if (bytesRangeEqualsSpecification.IsSatisfiedBy(tile2.Content)) return null;
if (changeType == ChangeType.ImageThematicalTile) {
return new ThematicalTileContainer((ThematicalBig1Tile) tile2.Clone());}
return new TileContainer((IContentTile) tile2.Clone(), changeType);
}
}
}
using MVCS.Diff4Big.Domain.ImageEntities;
using MVCS.Diff4Big.Domain.Specifications;
namespace MVCS.Diff4Big.Domain.Comparison.FT {
public class ByteByByte : ITileComparator {
private readonly LengthBased lengthBased = new LengthBased();
public IDeltaContainer Compare(IContentTile tile1, IContentTile tile2, ChangeType changeType) {
var bytesRangeEqualsSpecification = new BytesRangeEquals(tile1.Content);
var baseResult = lengthBased.Compare(tile1, tile2, changeType);
if (baseResult != null) return baseResult;
if (bytesRangeEqualsSpecification.IsSatisfiedBy(tile2.Content)) return null;
if (changeType == ChangeType.ImageThematicalTile) {
return new ThematicalTileContainer((ThematicalBig1Tile) tile2.Clone());
}
return new TileContainer((IContentTile) tile2.Clone(), changeType);
}
}
}
if (a == 0);
a++;
while(a++ != magic_number);
a = a << 2;
if (a == 0) {
a++;
}
while(a++ != magic_number) {}
a = a << 2;
Всегда оставляй лагерь чище, чем ты его нашел.
Вопросы?
hThreadArray[i] = CreateThread(
NULL, // default security attributes
0, // use default stack size
MyThreadFunction, // thread function name
pDataArray[i], // argument to thread function
0, // use default creation flags
&dwThreadIdArray[i]); // returns the thread identifier
wnd.CreateWnd(hInstance, wcname, NULL, WS_VISIBLE|WS_OVERLAPPEDWINDOW,
NWin::SRect(NWin::SPoint(CW_USEDEFAULT,CW_USEDEFAULT),
NWin::SSize(600,400)), NULL,
LoadMenu(hInstance, resWapp), NULL);