Оптимальное расположение кода в Mathematica?

Question

Я видел разные элегантные способы размещения кода на этом форуме.

Как новичок, я хотел бы получить ваши советы о том, как лучше расположить код, как показано ниже.

Это уродливо, не оптимально, возможно, даже глупо, но, может, опытные программисты простят меня, я использую это как "худший сценарий".

Цель новичка - ясность.

rejection[disp_, fixationNOtoConsiderForDuration_, durationLimit_, 
 minDistance_] :=

With[{fakedata = consider[t4dataLAEH10, 9, disp, {17, 18, 11}]},

With[{num = 
 Flatten[Position[
   Take[fakedata[[All, 3]], fixationNOtoConsiderForDuration], 
   x_ /; (x > durationLimit)]]},

If[num =!= {},
With[{fakedata1 = Drop[fakedata[[All, {1, 2}]], Last@num]},
 With[{num1 =
    Flatten[Position[
      Table[     
       Sqrt[((fakedata1[[fixation1, 1]] - 
           centerX)^2 + (fakedata1[[fixation1, 2]] - 
           centerY)^2)],
       {fixation1, 1, Length@fakedata1}], 
      x_ /; (x < minDistance)]]},

  If[num1 =!= {},
   Delete[fakedata1[[All, {1, 2}]], List /@ num1], 
   fakedata[[All, {1, 2}]]]]],
With[{fakedata2 = fakedata[[All, {1, 2}]]},  
 With[{num2 =
    Flatten[Position[
      Table[       
       Sqrt[((fakedata2[[fixation2, 1]] - 
           centerX)^2 + (fakedata2[[fixation2, 2]] - 
           centerY)^2)],
       {fixation2, 1, Length@fakedata2}], 
      x_ /; (x < minDistance)]]},
  If[num2 =!= {},
   Delete[fakedata2[[All, {1, 2}]], List /@ num2], 
   fakedata[[All, {1, 2}]]]]]]]]

Answer 1

• В случае кода выше, я бы разбил Flatten[Position[...]] на отдельную функцию.

• Я бы также использовал одинобзорная конструкция вместо вложенной With.

вроде:

Block[{fakedata, fakedata1, fakedata2, num, result},
  fakedata = ...;
  num = ...;

  If[num =!= {},
    fakedata1 = ...;
    result = localPoints[fakedata1];
    ];

  num1 = ...;
  If[num1 =!= {},
    fakedata2 = ...;
    result = localPoints[fakedata2];
    ];

  result
  ]

• Я не люблю использовать набор текста (индексы, надстрочные индексы, квадраткорни и т. д.) в коде, поскольку для меня это, как правило, текстовая среда (будь то Workbench, или электронная почта, или в Интернете, ...) В любом другом месте, это честная игра.

Answer 2

Я решил сделать полную переписку, потому что оригинал слишком далек от моего собственного стиля, чтобы я мог отформатировать его, как свой собственный код.

Я не пытался это проверить, поэтому вполне возможно, что что-то сломалось, но я считаю, что большая часть этого правильна.

Я использую текстовые ячейки для описания синтаксиса моих функций. Я обычно встраиваю (* comments *) для объяснения кода, но если объяснения становятся очень длинными, я также перемещаю их в текстовые ячейки.

Я включил комментарий, объясняющий func1. Это не очень полезный комментарий, но он служит примером.

Вот изображение моего ноутбука с увеличением 75%:

Выражение ячейки для копирования и вставки:

Cell[CellGroupData[{Cell["Rewrite", "Subsection"],

Cell[TextData[{
 StyleBox["distance", "Program"],
 "[{",
 StyleBox["x1",
  FontSlant->"Italic"],
 ", ",
 StyleBox["y1",
  FontSlant->"Italic"],
 "}] gives EuclideanDistance from {x1, y1} to global {centerX, \
centerY}\n",
 StyleBox["distance", "Program"],
 "[{",
 StyleBox["x1",
  FontSlant->"Italic"],
 ", ",
 StyleBox["y1",
  FontSlant->"Italic"],
 "}, {",
 StyleBox["x2",
  FontSlant->"Italic"],
 ", ",
 StyleBox["y2",
  FontSlant->"Italic"],
 "}] gives EuclideanDistance from {x1, y1} to {x2, y2}"
}], "Text"],

Cell[BoxData[
 RowBox[{
  RowBox[{"distance", "[", 
   RowBox[{"a_", ",", 
    RowBox[{"b_:", 
     RowBox[{"Hold", "@", 
      RowBox[{"{", 
       RowBox[{"centerX", ",", "centerY"}], "}"}]}]}]}], "]"}], ":=", 
  RowBox[{"EuclideanDistance", "[", 
   RowBox[{"a", ",", 
    RowBox[{"ReleaseHold", "@", "b"}]}], "]"}]}]], "Input"],

Cell[TextData[{
 StyleBox["rejection", "Program"],
 "[",
 StyleBox["disp, fixation, durationLimit, minDistance",
  FontSlant->"Italic"],
 "]\n\nfilters data from ",
 StyleBox["t4dataLAEH10", "Program"],
 " according to:\n\t",
 StyleBox["disp",
  FontSlant->"Italic"],
 " : (description of argument disp)\n\t",
 StyleBox["fixation",
  FontSlant->"Italic"],
 " : (description of argument fixation)\n\t",
 StyleBox["durationLimit",
  FontSlant->"Italic"],
 " : (description of durationLimit)\n\t",
 StyleBox["minDistance",
  FontSlant->"Italic"],
 " : (description of minDistance)"
}], "Text"],

Cell[BoxData[
 RowBox[{
  RowBox[{"rejection", "[", 
   RowBox[{
   "disp_", ",", "fixation_", ",", "durationLimit_", ",", 
    "minDistance_"}], "]"}], ":=", "\[IndentingNewLine]", 
  RowBox[{"Module", "[", 
   RowBox[{
    RowBox[{"{", 
     RowBox[{"fakedata", ",", "num", ",", "func1"}], "}"}], ",", 
    "\[IndentingNewLine]", 
    RowBox[{"(*", " ", 
     RowBox[{"description", " ", "of", " ", "fakedata"}], " ", "*)"}],
     "\[IndentingNewLine]", 
    RowBox[{
     RowBox[{"fakedata", "=", 
      RowBox[{"consider", "[", 
       RowBox[{"t4dataLAEH10", ",", "9", ",", "disp", ",", 
        RowBox[{"{", 
         RowBox[{"17", ",", "18", ",", "11"}], "}"}]}], "]"}]}], ";", 
     "\[IndentingNewLine]", "\[IndentingNewLine]", 
     RowBox[{"(*", " ", 
      RowBox[{"description", " ", "of", " ", "num"}], " ", "*)"}], 
     "\[IndentingNewLine]", 
     RowBox[{"num", "=", 
      RowBox[{"Position", "[", 
       RowBox[{
        RowBox[{
         RowBox[{"fakedata", "\[LeftDoubleBracket]", 
          RowBox[{"All", ",", "3"}], "\[RightDoubleBracket]"}], "~", 
         "Take", "~", "fixation"}], ",", 
        RowBox[{"x_", "/;", 
         RowBox[{"x", ">", "durationLimit"}]}]}], "]"}]}], ";", 
     "\[IndentingNewLine]", "\[IndentingNewLine]", 
     RowBox[{"(*", " ", 
      RowBox[{
       RowBox[{"func1", ":", " ", 
        RowBox[{
        "Take", " ", "the", " ", "first", " ", "two", " ", "columns", 
         " ", "of", " ", "fakedata"}]}], ",", " ", 
       RowBox[{
        RowBox[{
         RowBox[{
         "and", " ", "drop", " ", "rows", " ", "specified", " ", "by",
           " ", 
          RowBox[{
           StyleBox["dropspec",
            FontSlant->"Italic"], ".", "\[IndentingNewLine]", 
           "Delete"}], " ", "any", " ", "rows", " ", "for", " ", 
          "which", " ", 
          StyleBox["distance", "Program"]}], " ", "<", " ", 
         StyleBox["minDistance",
          FontSlant->"Italic"]}], ";", " ", 
        RowBox[{
        "if", " ", "no", " ", "rows", " ", "are", " ", "deleted"}]}], 
       ",", "\[IndentingNewLine]", "   ", 
       RowBox[{
       "return", " ", "the", " ", "first", " ", "two", " ", "columns",
         " ", "of", " ", "fakedata"}], ",", " ", 
       RowBox[{"ignoring", " ", 
        RowBox[{"dropspec", "."}]}]}], 
      StyleBox[" ",
       FontSlant->"Italic"], "*)"}], "\[IndentingNewLine]", 
     RowBox[{
      RowBox[{"func1", "[", "dropspec_", "]"}], ":=", 
      RowBox[{"Module", "[", 
       RowBox[{
        RowBox[{"{", 
         RowBox[{"part", ",", "fake"}], "}"}], ",", 
        "\[IndentingNewLine]", 
        RowBox[{
         RowBox[{"part", "=", 
          RowBox[{"fakedata", "\[LeftDoubleBracket]", 
           RowBox[{"All", ",", 
            RowBox[{"{", 
             RowBox[{"1", ",", "2"}], "}"}]}], 
           "\[RightDoubleBracket]"}]}], ";", "\[IndentingNewLine]", 
         RowBox[{"fake", "=", 
          RowBox[{"part", "~", "Drop", "~", "dropspec"}]}], ";", 
         "\[IndentingNewLine]", 
         RowBox[{
          RowBox[{
           RowBox[{"If", "[", 
            RowBox[{
             RowBox[{"#", "=!=", 
              RowBox[{"{", "}"}]}], ",", 
             RowBox[{"fake", "~", "Delete", "~", "#"}], ",", "part"}],
             "]"}], "&"}], "@", "\[IndentingNewLine]", 
          RowBox[{"Position", "[", 
           RowBox[{
            RowBox[{"distance", "/@", "fake"}], ",", 
            RowBox[{"x_", "/;", 
             RowBox[{"x", "<", "minDistance"}]}]}], "]"}]}]}]}], 
       "]"}]}], ";", "\[IndentingNewLine]", "\[IndentingNewLine]", 
     RowBox[{"If", "[", 
      RowBox[{
       RowBox[{"num", "=!=", 
        RowBox[{"{", "}"}]}], ",", 
       RowBox[{"func1", " ", "@", " ", 
        RowBox[{"num", "\[LeftDoubleBracket]", 
         RowBox[{
          RowBox[{"-", "1"}], ",", "1"}], "\[RightDoubleBracket]"}]}],
        ",", 
       RowBox[{"func1", "@", "0"}]}], "]"}]}]}], " ", 
   "]"}]}]], "Input"]
}, Open  ]]

Answer 3

У меня есть поговорка «не будь умным», то есть, когда это возможно, предпочти ясность кода выше других приоритетов.Когда-нибудь кто-нибудь еще раз посетит этот код, и он или она по достоинству оценят, что делает этот код.Это связано с форматированием (формат для наглядности), а также с вложением вызовов функций.В Mathematica легко написать очень непонятный код.Чтобы все было ясно, я склонен использовать временные или промежуточные переменные и разбивать свои длинные вложенные вызовы функций на маленькие значащие куски.Даже если вы повторно используете переменную, гораздо проще выполнить шаги или вставить туда Print [], чтобы увидеть, что происходит на каждом этапе.Вспомогательные функции тоже хороши.

С точки зрения форматирования, я склонен форматировать так:

   If[test, 
        expr1, 
   (* ELSE *)
        expr2
    ]

Мои предыдущие комментарии означают, что я склонен иметь несколько строк вместо множества длинных вложенных элементов, что также делает форматированиепроще.Старайтесь, чтобы expr1 / expr2 составляли менее 80 символов.Я думаю, что это разумная длина и достойный тест, если он становится слишком длинным.Это всего лишь руководящие принципы, которые могут и должны нарушаться время от времени, но я думаю, что они полезны, и надеюсь, что вы найдете их полезными тоже.

* ОБНОВЛЕНИЕ *

Так я бы просто отформатировал этот код .... Обратите внимание на довольно длинные строки ...

rejection[disp_, fixationNOtoConsiderForDuration_, durationLimit_, minDistance_] :=    
With[{fakedata = consider[t4dataLAEH10, 9, disp, {17, 18, 11}]},
    With[{num = Flatten[Position[Take[fakedata[[All, 3]], fixationNOtoConsiderForDuration], x_ /; (x > durationLimit)]]},

        If[num =!= {},
            With[{fakedata1 = Drop[fakedata[[All, {1, 2}]], Last@num]},
                With[{num1 = Flatten[Position[Table[Sqrt[((fakedata1[[fixation1, 1]] - centerX)^2 + (fakedata1[[fixation1, 2]] - centerY)^2)], {fixation1, 1, Length@fakedata1}], x_ /; (x < minDistance)]]},
                    If[num1 =!= {},
                        Delete[fakedata1[[All, {1, 2}]], List /@ num1],
                    (* ELSE *) 
                        fakedata[[All, {1, 2}]]
                    ]
                ]
            ],
        (* ELSE *)
            With[{fakedata2 = fakedata[[All, {1, 2}]]},  
                With[{num2 = Flatten[Position[Table[Sqrt[((fakedata2[[fixation2, 1]] - centerX)^2 + (fakedata2[[fixation2, 2]] - centerY)^2)], {fixation2, 1, Length@fakedata2}], x_ /; (x < minDistance)]]},
                    If[num2 =!= {},
                        Delete[fakedata2[[All, {1, 2}]], List /@ num2],
                    (* ELSE *) 
                        fakedata[[All, {1, 2}]]
                    ]
                ]
            ]
        ]
    ]
]

Более сложный рефакторинг:

(* Try to put types on these variables. It'll be easier to identify what's going in *)
rejection[disp_, fixationNOtoConsiderForDuration_, durationLimit_, minDistance_] := 
Module[{fakedata, num1, fakedata1, num, fakedata2, num2},
    fakedata = consider[t4dataLAEH10, 9, disp, {17, 18, 11}];
    num = Flatten[Position[Take[fakedata[[All, 3]], fixationNOtoConsiderForDuration], x_ /; (x > durationLimit)]];
    If[num =!= {}, 
        fakedata1 = Drop[fakedata[[All, {1, 2}]], Last@num];
        num1 = Table[Sqrt[((fakedata1[[fixation1, 1]] - centerX)^2 + (fakedata1[[fixation1, 2]] - centerY)^2)], {fixation1, 1, Length@fakedata1}];
        num1 = Flatten[Position[num1, x_ /; (x < minDistance)]];
        If[num1 =!= {},
            Delete[fakedata1[[All, {1, 2}]], List /@ num1],
        (* ELSE *) 
            fakedata[[All, {1, 2}]]
        ], 
    (* ELSE *)
        fakedata2 = fakedata[[All, {1, 2}]];
        num2 = Table[Sqrt[((fakedata2[[fixation2, 1]] - centerX)^2 + (fakedata2[[fixation2, 2]] - centerY)^2)], {fixation2, 1, Length@fakedata2}];
        num2 = Flatten[Position[num2, x_ /; (x < minDistance)]];
        If[num2 =!= {},
            Delete[fakedata2[[All, {1, 2}]], List /@ num2],
        (* ELSE *) 
            fakedata[[All, {1, 2}]]
        ]           
    ]
]

Здесь больше переменных, чем необходимо, и меня не устраивает строка num1 / 2 = Table [].Это все еще долго, и я, вероятно, либо попытался бы сделать из него вспомогательную функцию, либо использовал бы Normal [], что похоже на то, что пытается сделать этот код.