私たちは数学が精度の科学であることを知っています。インタラクティブな数学学習ソフトウェアである GeoGebra についても同じことが言えますか? PVS-Studioを使ってプロジェクトのソースコードを解析してみましょう!
大学でコンピューター サイエンスをどのように学んだか覚えていますか?これらすべての行列とベクトルの乗算、多項式、内挿、外挿...これらの恐ろしい数式を、別の実験レポートではなく実際のプロジェクトで見たらどうなるでしょうか?このようなコードベースの問題を掘り下げてみるとどうなるでしょうか? PVS-Studio を実行して、数学の教科書の埃を払うことをお勧めします。なぜ教科書なのか？見せてあげましょう
このようなプログラムのソース コードを調査する際の主な課題の 1 つは、何が起こっているかを理解することです。アナライザー レポートを確認する際、警告が実際の問題を示しているのかどうかという疑問がありました。
次の断片を見てみましょう:
次の PVS-Studio 警告が表示されます:
V6072 2 つの類似したコードの断片が見つかりました。おそらく、これはタイプミスであり、「a」の代わりに「b」変数を使用する必要があります。 AlgoTriangularDF.java 145、AlgoTriangularDF.java 146、AlgoTriangularDF.java 147、AlgoTriangularDF.java 148
本当にタイプミスですか？簡単な調査の結果、正しい公式が見つかったら、すべてが正しく記述されていると言えます。
コード部分は三角分布、つまりこの分布の確率密度関数 (PDF) を評価します。次の式が見つかりました:
それでは、コードを見てみましょう。
ここで* fv* は関数変数です。wrapはwrapperを返し、その後、必要な数学的演算が実行されます。multiplyメソッドとmultiplyRメソッドの両方があることに注目するのは興味深いことです。 2 番目の方法では、乗算が常に可換であるとは限らないため、Rはrightを表し、オペランドを交換します。
したがって、2 番目の式の結果はbranchAToModeに書き込まれ、4 番目の式の結果はbranchModeToBに書き込まれます。また、
branchModeToBで分子と分母の符号が変更されていることにも気付きました。次の式が得られます:
式の値は変更されませんでした。
そこで、私たちは受け取った警告のいくつかを理解するために数学的知識を更新しました。コードに実際のエラーがあるかどうかを特定するのは難しくありませんが、代わりにここに何が存在するのかを理解するのは困難です。
エラー
リーリー
誰かが
をs[3]に置き換えるのを忘れていることがわかります。最後の行の効果はその輝きにあります。これは伝説的で、あまりにも一般的なコピー＆ペーストのエラーです。その結果、4 番目の配列項目が欠落しており、null!になります。V6033 同じキー「2」を持つ項目はすでに変更されています。 AlgoPolyhedronNetPrism.java 376、AlgoPolyhedronNetPrism.java 377
価値観についてはどうでしょうか？
リーリー
素晴らしい景色ですね！読むのは楽しいですが、このメソッドは 66 行目で始まり 404 行目で終わるため、これはほんの一部に過ぎません。アナライザーは、V6033 タイプの警告を 50 件発行します。これらの警告の 1 つを簡単に見てみましょう:
V6033 同じキー「〜」を持つ項目が既に追加されています。 MathMLParser.java 229、MathMLParser.java 355
余分な断片を削除して、警告に言及している表現を見てみましょう:
リーリー
でも、面白いですよ。メソッド呼び出し間の間隔はどれくらいですか? 126行あります。そうですね、このようなエラーを手作業で見つけることができれば幸いです!
ほとんどはキーと値が重複しています。ただし、いくつかのケースは上記の例と似ており、開発者が値を別の値で上書きします。どれを使えばいいでしょうか？
円または楕円
リーリーメソッドもあります。では、それは何でしょうか？もう少し詳しく見てみましょうすべては
DrawConic3Dクラスで行われます。楕円と円のメソッドは次のとおりです:
protected void updateCircle(PlotterBrush brush) { if (visible == Visible.CENTER_OUTSIDE) { longitude = brush.calcArcLongitudesNeeded(e1, alpha, getView3D().getScale()); brush.arc(m, ev1, ev2, e1, beta - alpha, 2 * alpha, longitude); } else { longitude = brush.calcArcLongitudesNeeded(e1, Math.PI, getView3D().getScale()); brush.circle(m, ev1, ev2, e1, longitude); } } protected void updateEllipse(PlotterBrush brush) { if (visible == Visible.CENTER_OUTSIDE) { brush.arcEllipse(m, ev1, ev2, e1, e2, beta - alpha, 2 * alpha); } else { brush.arcEllipse(m, ev1, ev2, e1, e2, 0, 2 * Math.PI); } }
Well... It doesn't give that much confidence. The method bodies are different, but nothing here indicates that we risk displaying unacceptable geometric objects if the method is called incorrectly.
Could there be other clues? A whole single one! TheupdateCirclemethod is never used in the project. Meanwhile,updateEllipseis used four times: twice in the first fragment and then twice inDrawConicSection3D, the inheritor class of* DrawConic3D*:
@Override protected void updateCircle(PlotterBrush brush) { updateEllipse(brush); } @Override protected void updateEllipse(PlotterBrush brush) { // .... } else { super.updateEllipse(brush); } }
ThisupdateCircleisn't used, either. So,updateEllipseonly has a call in its own override and in the fragment where we first foundupdateForItSelf. In schematic form, the structure looks like as follows:
On the one hand, it seems that the developers wanted to use the all-purposeupdateEllipsemethod to draw a circle. On the other hand, it's a bit strange thatDrawConicSection3Dhas theupdateCirclemethod that callsupdateEllipse. However,updateCirclewill never be called.
It's hard to guess what the fixed code may look like if the error is actually in the code. For example, ifupdateCircleneeds to callupdateEllipseinDrawConicSection3D, butDrawConic3Dneeds a more optimized algorithm for the circle, the fixed scheme might look like this:
So, it seems that developers once wroteupdateCircleand then lost it, and we may have found its intended "home". Looks like we have discovered the ruins of the refactoring after which the developers forgot about the "homeless" method. In any case, it's worth rewriting this code to make it clearer so that we don't end up with so many questions.
All these questions have arisen because of the PVS-Studio warning. That's the warning, by the way:
V6067 Two or more case-branches perform the same actions. DrawConic3D.java 212, DrawConic3D.java 215
private void updateOrdering(GeoElement geo, ObjectMovement movement) { .... switch (movement) { .... case FRONT: .... if (index == firstIndex) { if (index != 0) { geo.setOrdering(orderingDepthMidpoint(index)); } else { geo.setOrdering(drawingOrder.get(index - 1).getOrdering() - 1); } } .... } .... }
We get the following warning:
V6025 Index 'index - 1' is out of bounds. LayerManager.java 393
This is curious because, in theelseblock, theindexvariable is guaranteed to get the value 0. So, we pass -1 as an argument to thegetmethod. What's the result? We catch anIndexOutOfBoundsException.
@Override protected int getGLType(Type type) { switch (type) { case TRIANGLE_STRIP: return GL.GL_TRIANGLE_STRIP; case TRIANGLE_FAN: return GL.GL_TRIANGLE_STRIP; // <= case TRIANGLES: return GL.GL_TRIANGLES; case LINE_LOOP: return GL.GL_LINE_LOOP; case LINE_STRIP: return GL.GL_LINE_STRIP; } return 0; }
The code is new, but the error is already well-known. It's quite obvious thatGL.GL_TRIANGLE_STRIPshould beGL.GL_TRIANGLE_FANinstead*.* The methods may be similar in some ways, but the results are different. You can read about it under the spoiler.
V6067 Two or more case-branches perform the same actions. RendererImplShadersD.java 243, RendererImplShadersD.java 245
To describe a series of triangles, we need to save the coordinates of the three vertices of each triangle. Thus, given N triangles, we need the saved 3N vertices. If we describe a polyhedral object using a polygon mesh, it's important to know if the triangles are connected. If they are, we can use the Triangle Strip or the Triangle Fan to describe the set of triangles using N + 2 vertices.
We note that the Triangle Fan has been removed in Direct3D 10. In OpenGL 4.6, this primitive still exists.
The Triangle Fan uses one center vertex as common, as well as the last vertex and the new vertex. Look at the following example:
To describe it, we'd need the entry (A, B, C, D, E, F, G). There are five triangles and seven vertices in the entry.
The Triangle Strip uses the last two vertices and a new one. For instance, we can create the image below using the same sequence of vertices:
Therefore, if we use the wrong primitive, we'll get dramatically different results.
public static void addToJsObject( JsPropertyMap
If we don't look closely, we may not notice the issue right away. However, let's speed up and just check the analyzer message.
V6086 Suspicious code formatting. 'else' keyword is probably missing. ScriptManagerW.java 209
Finally, a more interesting bug is here. Theobject instanceof String[]check will occur regardless of the result ofobject instanceof Integer. It may not be a big deal, but there's also theelseblock that will be executed after a failed check forString[]. As the result, thejsMapvalue byentry.getKey()will be overwritten if theobjectwasInteger.
There's another similar case, but the potential consequences are a little harder to understand:
public void bkspCharacter(EditorState editorState) { int currentOffset = editorState.getCurrentOffsetOrSelection(); if (currentOffset > 0) { MathComponent prev = editorState.getCurrentField() .getArgument(currentOffset - 1); if (prev instanceof MathArray) { MathArray parent = (MathArray) prev; extendBrackets(parent, editorState); } if (prev instanceof MathFunction) { // <= bkspLastFunctionArg((MathFunction) prev, editorState); } else { // <= deleteSingleArg(editorState); } } else { RemoveContainer.withBackspace(editorState); } }
V6086 Suspicious code formatting. 'else' keyword is probably missing. InputController.java 854
Do you often have to write many repetitious checks? Are these checks prone to typos? Let's look at the following method:
public boolean contains(BoundingBox other) { return !(isNull() || other.isNull()) && other.minx >= minx && other.maxy <= maxx && other.miny >= miny && other.maxy <= maxy; }
V6045 Possible misprint in expression 'other.maxy <= maxx'. BoundingBox.java 139
We find a typo, we fix it. The answer is simple, there should beother.maxx <= maxx.
public PointDt[] calcVoronoiCell(TriangleDt triangle, PointDt p) { .... // find the neighbor triangle if (!halfplane.next_12().isHalfplane()) { neighbor = halfplane.next_12(); } else if (!halfplane.next_23().isHalfplane()) { neighbor = halfplane.next_23(); } else if (!halfplane.next_23().isHalfplane()) { // <= neighbor = halfplane.next_31(); } else { Log.error("problem in Delaunay_Triangulation"); // TODO fix added by GeoGebra // should we do something else? return null; } .... }
Let's see the warning:
V6003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. DelaunayTriangulation.java 678, DelaunayTriangulation.java 680
We don't even need to figure out what happens to half-planes, because it's clear that the!halfplane.next_31().isHalfplane()check is missing.
public static ExpressionNode get( ExpressionValue left, ExpressionValue right, Operation operation, FunctionVariable fv, Kernel kernel0 ) { .... switch (operation) { .... case VEC_FUNCTION: break; case ZETA: break; case DIRAC: return new ExpressionNode(kernel0, left, Operation.DIRAC, fv); case HEAVISIDE: return new ExpressionNode(kernel0, left, Operation.DIRAC, fv); default: break; } .... }
It seems that, in the second case,Operation.DIRACshould probably be replaced withOperation.HEAVISIDE. Or not? This method is called to obtain the derivative. After researching, we understand whatHEAVISIDE *is—the use of *Operation.DIRACfor it is correct. What about theDiracderivative? This one is a bit tricky to understand. We may trust the developers and suppress the warning. Still, it'd be better if they'd left any explanatory comment as they had done in some cases before.
case FACTORIAL: // x! -> psi(x+1) * x! return new ExpressionNode(kernel0, left.wrap().plus(1), Operation.PSI, null) .multiply(new ExpressionNode(kernel0, left, Operation.FACTORIAL, null)) .multiply(left.derivative(fv, kernel0)); case GAMMA: // gamma(x) -> gamma(x) psi(x) return new ExpressionNode(kernel0, left, Operation.PSI, null) .multiply(new ExpressionNode(kernel0, left, Operation.GAMMA, null)) .multiply(left.derivative(fv, kernel0));
And we were motivated to conduct such research by the message of already favorite diagnostic V6067:
V6067 Two or more case-branches perform the same actions. Derivative.java 442, Derivative.java 444
On the one hand, this is an interesting case because we could have a false positive here. On the other, the warning would be useful either way, as it often highlights a genuine error. Regardless of the results, we need to check such code and either fix the error or write explanatory comments. Then the warning can be suppressed.
private static GeoElement doGetTemplate(Construction cons, GeoClass listElement) { switch (listElement) { case POINT: return new GeoPoint(cons); case POINT3D: return (GeoElement) cons.getKernel().getGeoFactory().newPoint(3, cons); case VECTOR: return new GeoVector(cons); case VECTOR3D: return (GeoElement) cons.getKernel().getGeoFactory().newPoint(3, cons); } return new GeoPoint(cons); }
The warning isn't hard to guess:
V6067 Two or more case-branches perform the same actions. PointNDFold.java 38, PointNDFold.java 43
Instead ofcons.getKernel().getGeoFactory().newPoint(3, cons)in the second case,cons.getKernel().getGeoFactory().newVector(3, cons)may have been intended. If we want to make sure of it, we need to go deeper again. Well, let's dive into it.
So,* getGeoFactory()returnsGeoFactory,let's look at the *newPoint *andnewVector* methods:
public GeoPointND newPoint(int dimension, Construction cons) { return new GeoPoint(cons); } public GeoVectorND newVector(int dimension, Construction cons) { return new GeoVector(cons); }
It looks a bit strange. Thedimensionargument isn't used at all. What's going on here? Inheritance, of course! Let's find theGeoFactory3Dinheritor class and see what happens in it.
@Override public GeoVectorND newVector(int dimension, Construction cons) { if (dimension == 3) { return new GeoVector3D(cons); } return new GeoVector(cons); } @Override public GeoPointND newPoint(int dimension, Construction cons) { return dimension == 3 ? new GeoPoint3D(cons) : new GeoPoint(cons); }
Excellent! Now we can admire the creation of four different objects in all their glory. We return to the code fragment with the possible error. ForPOINTandPOINT3D, the objects of theGeoPointandGeoPoint3Dclasses are created.GeoVectoris created forVECTOR, but poorGeoVector3Dseems to have been abandoned.
Sure, it's a bit strange to use a factory method pattern in two cases and call constructors directly in the remaining two. Is this a leftover from the refactoring process, or is it some kind of temporary solution that'll stick around until the end of time? In my opinion, if the responsibility for creating objects has been handed over to another class, it'd be better to fully delegate that responsibility.
@Override final public void update() { .... switch (angle.getAngleStyle()) { .... case EuclidianStyleConstants.RIGHT_ANGLE_STYLE_L: // Belgian offset |_ if (square == null) { square = AwtFactory.getPrototype().newGeneralPath(); } else { square.reset(); } length = arcSize * 0.7071067811865; // <= double offset = length * 0.4; square.moveTo( coords[0] + length * Math.cos(angSt) + offset * Math.cos(angSt) + offset * Math.cos(angSt + Kernel.PI_HALF), coords[1] - length * Math.sin(angSt) * view.getScaleRatio() - offset * Math.sin(angSt) - offset * Math.sin(angSt + Kernel.PI_HALF)); .... break; } }
Where's the warning? There it is:
V6107 The constant 0.7071067811865 is being utilized. The resulting value could be inaccurate. Consider using Math.sqrt(0.5). DrawAngle.java 303
Nous avons trouvé quatre instances de ce fragment de code. La valeur la plus précise est 0,7071067811865476. Relevez le défi et essayez de l'apprendre par cœur, hehe. Les trois derniers chiffres sont omis. Est-ce critique ? La précision est probablement suffisante, mais l'utilisation de constantes prédéfinies ou deMath.sqrt(0.5)— comme dans ce cas — aidera non seulement à récupérer les chiffres perdus, mais éliminera également le risque de fautes de frappe. Remarquez comment la lisibilité du code s'améliorera. Peut-être qu'en voyant 0,707... quelqu'un comprend immédiatement de quel genre de nombre magique il s'agit. Cependant, nous pouvons parfois faire un peu moins de magie simplement pour améliorer la lisibilité du code.
Notre petit voyage mathématique est terminé. Comme nous pouvons le constater, même après avoir relevé de nombreux défis géométriques, les gens peuvent encore commettre de simples erreurs de code ! Bien si le code est récent et que les développeurs peuvent toujours garder à l'esprit leurs intentions passées. Dans de tels cas, les gens peuvent comprendre quels changements sont nécessaires, le cas échéant, mais après une longue période, il peut ne pas être si facile de reconnaître la nécessité de résoudre ces problèmes.
Ainsi, l'analyseur s'avère assez efficace dès l'écriture du code plutôt que de l'exécuter une seule fois comme je l'ai fait. Surtout maintenant que nous disposons d'outils comme l'analyseur PVS-Studio.
위 내용은 훌륭한 수학자도 실수를 한다의 상세 내용입니다. 자세한 내용은 PHP 중국어 웹사이트의 기타 관련 기사를 참조하세요!
![[웹 프런트엔드] Node.js 빠른 시작](https://img.php.cn/upload/course/000/000/067/662b5d34ba7c0227.png)
