This statment: <div class="Box-header

Try this: <div class="snippet-clipboard-content notranslate position-relative over

Mmm, nope. Why it would be? Then <code cl

Useless statement about nanomorph HOT 8 OPEN

choojs commented on May 14, 2024

Useless statement

from nanomorph.

Comments (8)

goto-bus-stop commented on May 14, 2024

I think if the condition on the next line is true, the function can still return true.

from nanomorph.

tunnckoCore commented on May 14, 2024

Ordering of the ifs are pretty important. That's why. So, no, it's not useless.

from nanomorph.

c-smile commented on May 14, 2024

Consider this:

function same(a,b) is called for a and b of Element type so effectively you get this:

function same (a, b) {
  ...
  if (a.tagName !== b.tagName) return false; // either false
  // if (a.type === TEXT_NODE) return a.nodeValue === b.nodeValue - dead statement if Elements
  return false; // or false again
}

If a and b are Node's then a.tagName is always undefined thus that part looks as this

function same (a, b) {
  ...
  // if (a.tagName !== b.tagName) return false; - dead statment if Nodes 
  if (a.type === TEXT_NODE) return a.nodeValue === b.nodeValue;
  return false;
}

from nanomorph.

tunnckoCore commented on May 14, 2024

Even if this makes sense, still. The thing is that we don't know what a and b are at the point where we call the same(). In one case it can be Element, in another, it can be Node. It's not completely useless or just sitting there, never used. I'm pretty sure it is covered when you run the coverage.

There are tons of tests. I also had reimplemented it (in demo-minmorph) a dozen of times just out of curiosity.

from nanomorph.

c-smile commented on May 14, 2024

Try this:

<html>
    <head>
        <title>Test</title>
    </head>
    <body>
    <div>1</div>
    <div>2</div>
    </body>
<script type="text/javascript">

function same (a, b) {
  if (a.id) return a.id === b.id;
  if (a.isSameNode) return a.isSameNode(b);
  if (a.tagName !== b.tagName) return false;
  if (a.type === TEXT_NODE) return a.nodeValue === b.nodeValue;
  return false;
}

var divs = document.body.getElementsByTagName("div");
console.log( same(divs[0],divs[1]) );
</script>

</html>

You will see false in console. But I think true is expected there, right?

from nanomorph.

tunnckoCore commented on May 14, 2024

But I think true is expected there, right?

Mmm, nope. Why it would be? It goes to the a.nodeValue === b.nodeValue which should return false, because it's 1 === 2.

I'll say that again, same is called recursively anyway, so 1) we don't know what's what in a given point in time and 2) it makes sense to have exactly these checks.

from nanomorph.

c-smile commented on May 14, 2024

Mmm, nope. Why it would be?

Then if (a.tagName !== b.tagName) return false; is useless.

QED.

from nanomorph.

tunnckoCore commented on May 14, 2024

Omg, in this exact case, yes of course. Remove it and run the tests, and you will see that we need it.

Even if we remove it or put it in separate function... We still need to have this check anyway. That's why it is there in the same.

I'm done here.

from nanomorph.

Useless statement about nanomorph HOT 8 OPEN

Comments (8)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent