This week's book giveaway is in the Reactive Progamming forum.
We're giving away four copies of Reactive Streams in Java: Concurrency with RxJava, Reactor, and Akka Streams and have Adam Davis on-line!
See this thread for details.
Win a copy of Reactive Streams in Java: Concurrency with RxJava, Reactor, and Akka Streams this week in the Reactive Progamming forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Junilu Lacar
  • Jeanne Boyarsky
  • Bear Bibeault
Sheriffs:
  • Knute Snortum
  • Tim Cooke
  • Devaka Cooray
Saloon Keepers:
  • Ron McLeod
  • Stephan van Hulst
  • Tim Moores
  • Tim Holloway
  • Carey Brown
Bartenders:
  • Piet Souris
  • Frits Walraven
  • Ganesh Patekar

Chess problem: calculating the intersection point of two bishops with simple equations

 
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello Ranchers!

EDIT: I forgot the link of the problem!
https://open.kattis.com/problems/chess


I am back to school so I have less time for hobbies , but I miss you and my cows (in that order) a lot, and I hope you miss me a little bit too .

Anyway, this morning I tried to solve a problem : can two bishops on a board attack each other or not?
There are several cases:
- either they have different colors (if one bishop's coordinates add up to an even number, and the second bishop coordinates add up to an odd number). In that case it's impossible.
- they have the same coordinates: zero moves
- they are on the same line: we draw a line between (x1, y1) and (x2, y2) and check if the slope is one: it's one move.
- last case:
Since bishops move in diagonals with slope 1 or -1, I came up with the idea that they should always be able to go one direction and move in the other (2 moves). The problem specifies that we have up to four moves, but hey, why wasting moves?
I gave the "attacker" the line with slope k = -1, and the "defender" the slope with k = 1 and calculated the intersection x, y. Shall x and y be out of range, then i reverted the lines. The "attacker" has now the slope with k = 1, and the "defender" the line with the slope k = -1. Have a look at my drawing!
The image sharing does not seem to work, so I add it in attachment.





I wrote 31 test cases* for the borders and inside the chessboard and it works. But unfortunately I fail with the judge.
I cannot find the bug  

And since I love you all Ranchers very much, I wrote all in Java and tried to do all in a Javanic way  . I even closed the scanner.

*not automated. Why not, even after Junilu told me dozens times to do so?
Because I am an idiot. That being established, please have a look at my dribble.



Those are my tests, that end up correct.
fieldstudy.jpg
[Thumbnail for fieldstudy.jpg]
 
Saloon Keeper
Posts: 10657
227
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Good to have you back!

D.J. Quavern wrote:I wrote all in Java and tried to do all in a Javanic way  


I'm afraid that apart from the syntax and the classes from the standard API, there's not much 'Javanic' about your code. Why is everything in the main method? Why have you not split up your application in separate classes that are responsible for different things? Why are you using poor variable names like fB and sB?

First, I suggest you rewrite the application so that you have a Position class that encapsulates the coordinates on a chessboard, a Diagonal class that can tell you the positions of the squares that are on the diagonal it represents, and a Bishop class that encapsulates a position and can determine in how many moves it can reach another specified position. Then, in your main class add a method that reads a position from the standard input. Call it twice and construct a bishop of one of them. Let the bishop determine how many moves it needs to reach the other position.

Now that you have different classes and methods, you can write proper unit tests that will tell you in what part of the code the problem lies.

Before you implement any methods, write some skeleton classes and show it to us so we can tell you you are on the right track. For instance, you can add a method getColor() to the Position class, but don't implement it yet.

I even closed the scanner.


You managed to do it at the one time you shouldn't have: don't close System.in or resources based on System.in. Also, when you do want to close a resource, do it by using the try-with-resources syntax.
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:Good to have you back!

D.J. Quavern wrote:I wrote all in Java and tried to do all in a Javanic way  


I'm afraid that apart from the syntax and the classes from the standard API, there's not much 'Javanic' about your code. Why is everything in the main method? Why have you not split up your application in separate classes that are responsible for different things? Why are you using poor variable names like fB and sB?

First, I suggest you rewrite the application so that you have a Position class that encapsulates the coordinates on a chessboard, a Diagonal class that can tell you the positions of the squares that are on the diagonal it represents, and a Bishop class that encapsulates a position and can determine in how many moves it can reach another specified position. Then, in your main class add a method that reads a position from the standard input. Call it twice and construct a bishop of one of them. Let the bishop determine how many moves it needs to reach the other position.



Haha I deserved that.

I did not think I need several methods for a problem which is so short. But obviously if I had done so I would have a way to narrow down where the bug could lie.

But one thing I have no idea how to implement is the diagonal. What do you mean by that?

PS: you did not happen to see the bug on your way out?




 
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

D.J. Quavern wrote:
I did not think I need several methods for a problem which is so short. But obviously if I had done so I would have a way to narrow down where the bug could lie.


You wrote:

The comment "are they the same color" implies that's what the conditional expression means. It's not. At best, that makes your code misleading to the reader.

If you had extracted the expression to an aptly-named method in the Position class, the code could be much clearer, more object-oriented, and you wouldn't need the comment:

This kind of thing, where your comment says one thing but the code says another is the source of many bugs.

This is just one example of how that code would benefit from being made more modular, more OO, clearer, and more expressive.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The quest for simplicity/modularity/clarity doesn't end with the first refactoring you perform though. I suggested this refactoring:

However, if I were refactoring ruthlessly, I'd probably follow the path suggested by Stephan and end up with something like this:

Which means that I might have something like this:
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here's another example of where you seem to go out of your way to write cryptic code:

The comments at the end of line are well-meant but you could have gone the extra step and saved the reader of the extra effort of translating "fB" to "firstBishop" all throughout the rest of the code. It's called "reducing cognitive load" or as I say, "Being nice to the reader."

First refactoring: rename to clarify intent

Second refactoring: simplify API and clarify intent:

As I push implementation code down and make intent-ful code bubble up and clarify the driving logic, I'd also look for other opportunities to refactor. For example, you have this kind of cryptic expression: line[0] - 65. The meaning/intent of that is not readily apparent. It would be clearer if you wrote this: line[0] - 'A' but for me, that still wouldn't be enough. (I guess by now you get a sense of how ruthlessly I refactor for clarity in my code)

Note: Each refactoring has to be supported by tests. It is while writing these tests that you are likely to find the bug in your code.
 
Stephan van Hulst
Saloon Keeper
Posts: 10657
227
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Unless the thing you want to solve can be solved in two lines of code, take the time to design a proper object oriented model of the problem.

After thinking about it a bit more, you probably don't need a Diagonal class. Instead, add a method isOnSameDiagonal(Position other) to your Position class. Your problem then reduces to finding a position that's on the same diagonal as the original position of the bishop and is also on the same diagonal as the target position.
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:Unless the thing you want to solve can be solved in two lines of code, take the time to design a proper object oriented model of the problem.

After thinking about it a bit more, you probably don't need a Diagonal class. Instead, add a method isOnSameDiagonal(Position other) to your Position class. Your problem then reduces to finding a position that's on the same diagonal as the original position of the bishop and is also on the same diagonal as the target position.



Yes, but this is what I address in the "else" case.
If they are of the same color, if they are not the same object and if they are not on the same diagonal, then they can reach each other in two moves.

@Junilu and everyone: yes, I agree, it is a bit cryptic, and some comments I made to you are weird (especially the color one: this tests if both bishops are on different colors. If yes, the take is impossible). But since it is such a small piece of code I did not think I needed so many clarifications.

Last edit : besides, my tests give me right results. Can you think about a test that could fail?
 
Stephan van Hulst
Saloon Keeper
Posts: 10657
227
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

D.J. Quavern wrote:Yes, but this is what I address in the "else" case.


Well, it's just not very clear, or object oriented. Any time you have to write a piece of comment inside a method body to explain what a statement or expression is doing, then you should take it as a hint to make a method out of it instead. Preferably, the method should be an instance method on an appropriate type, and if you don't have an appropriate type that you can add the method to, it's probably time to think about creating a new class.

But since it is such a small piece of code I did not think I needed so many clarifications.


A single clarification is one clarification too much. Code should be self documenting, meaning that small bits of code should be inside a method with a name that very clearly says what that piece of code is supposed to do, and variable names should clearly and correctly state what they represent. Abbreviations are almost never good variable names. Take another look at the boolean expression that Junilu replaced with a method call to firstPosition.isDifferentColor(secondPosition). It's immediately clear what the method is supposed to do, without having to decrypt the actual expression that you used. That's because he took the time to write out the variable and method names completely and correctly.

besides, my tests give me right results. Can you think about a test that could fail?


Show us the latest version of your code, hopefully refactored somewhat to include some of our suggestions.
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Ranchers,
Here comes some hideous refactoring

Edit: I did some ugly tests. You can still help me to write better tests if you wish so, but now I would really, really, really like to find the bug.



Can I also please have some help to write a test?
I have those:


Initialized as such:


and I don't know how to tell junitest that I want the return value to be B5:

 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
And here comes some tests:

 
Marshal
Posts: 65782
250
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Where have you got Piece classes, including King Queen etc?
Why have you got so many static methods?
What does something like new Main.BishopPosition(...) mean? Have you really got static nested classes? Why doesn't each Piece record its own position, and have methods to move it?

Can you calculate the colour of a square by adding its letter (file) and number (rank)?
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thank you everyone!

I got it.

It was an offset at
instead of . Since I added "'A'+ char" before printing, it was impossible to detect it. So I wrote a zero-based code, when everything was 1-based (I thought it was annoying to do a zero-based system when we were calculating right lines equations anyway). By zero based system I mean that counting begins at zero like arrays.
Since I had a 1-based system, I should have substracted 64, instead of 65. And obviously added back 64.

When I refactored my code as you demanded advised, I could see that some tests were failing, and the offset became apparent. So, yeah, I could have not understood without doing the it the correct way.

I know what you think, "will DJ learn this time...?"

 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Campbell Ritchie wrote:
Where have you got Piece classes, including King Queen etc?


The problem only has bishops that want to kill each other. I did not need any more murderous royalty.

Why have you got so many static methods?



Because Java would not compile.

What does something like new Main.BishopPosition(...) mean?


I have no idea. I did stuff until Java accepted to compile. I had this BishopPosition class nested in main, so I needed apparently to instantiate Main to be able to call BishopPosition.

Have you really got static nested classes?  


I don't know what it means, I'm sorry. My understanding of Java is very superficial.

Why doesn't each Piece record its own position, and have methods to move it?


Why does it need to?


Can you calculate the colour of a square by adding its letter (file) and number (rank)?


Yes you can! I needed to google that part, but it works. It kind of makes sense in an intuitive way, since the color changes every two steps, horizontal or vertical.


 
Campbell Ritchie
Marshal
Posts: 65782
250
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

D.J. Quavern wrote:. . . "will DJ learn this time...?"

Even if not now, she will learn it eventually.
 
Sheriff
Posts: 6266
167
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

D.J. Quavern wrote:[

Why have you got so many static methods?



Because Java would not compile.


That's because you still have too much code in the main(String[]) method.  Se MainIsAPain (that a link).
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

D.J. Quavern wrote:
It was an offset at
instead of . Since I added "'A'+ char" before printing, it was impossible to detect it.


Obviously, not impossible because you eventually did. Cryptic code like this usually results in bugs and you've shown again how true that is.

I think I'll start a thread in the Refactoring forum to walk through the refactoring of your code so that 1) The code can be put under tests, 2) the code can be refactored safely, 3) the bug can be revealed by failing tests, and 4) the bug can finally be addressed. This is a more systematic and disciplined way of dealing with bugs and has the benefit of ending up with cleaner code that is supported by tests.

I know what you think, "will DJ learn this time...?"


Don't know, that's really up to you. But follow along here if you want to learn how to write better programs: https://coderanch.com/t/716893/engineering/Refactoring-Exercise
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I will do some more refactoring, like adding methods you suggested to the BishopPosition class.
On the other hand, I understand the concept with main is a pain, but I still don't understand how static really work and how I can take away all static methods I have here.
 
Stephan van Hulst
Saloon Keeper
Posts: 10657
227
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Have you gone through the Java tutorials, and do you know what an object is?
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Stephan van Hulst wrote:Have you gone through the Java tutorials, and do you know what an object is?



No, I did not go though tutorials, but had Java as first language at school. I learned about objects and OOP, but not how to organize objects in static /dynamic classes. We then moved on to algorithms.
 
Ranch Hand
Posts: 43
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You don't have a coding problem.

You have a modeling problem. You've already identified the solution to "Can two bishops attack" translates to are they on the same diagonal?

So the question becomes -- how do you model the diagonal of a chess square? Assuming standard algebraic chess notation given in the link, if we increase both the column letter and the row number by one, we're on the same diagonal. Alternatively, if we increase the column letter and decrease row by one, we're on the same diagonal. So if the sum or difference of the row and column coordinates are the  same, the pieces are on the same diagonal.

The code code then like something like this:

Piece is an immutable class that models the row and column as integers. Representation seemed easier than using the row letter (character), but I could see storing the a to g letter instead of the column number.

Diagonals is a simple immutable return class for the diagonals method of Piece, which calculates the two diagonals as the sum and difference as the coordinates (line 60). Two diagonals intersect if either of the two values are equal.

Then Bishop becomes straightforward.


and a test case

 
Knute Snortum
Sheriff
Posts: 6266
167
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Looks good!

I think you can simplify the Diagonal class to one field, and use

Also, in the Piece class, I'd add after line 38:
I haven't tested any of this, though.
 
Gerard Charles
Ranch Hand
Posts: 43
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Knute Snortum wrote:Looks good!

I think you can simplify the Diagonal class to one field, and use

Also, in the Piece class, I'd add after line 38:
I haven't tested any of this, though.



Don't see how Diagonals could work with a single field, since each square is on two diagonals.

Concur the IllegalArgumentException could be improved.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:You don't have a coding problem. You have a modeling problem. ... So the question becomes -- how do you model the diagonal of a chess square


Thanks for sharing your approach. It certainly is interesting. I find it fascinating, actually, to study how others approach problems and try to imagine how the gears in their heads are turning and meshing to produce different representations from the same input.

OP, for example, interpreted the requirements as two Bishops battling each other when, in fact, the original requirements say nothing or even allude to anything of that sort. It only says "Given a chess board and the standard bishop moves, figure out how a bishop might get from one position on the board to another."

In your model, you have introduced the idea of a Piece which is somehow closely related to Diagonal (I assume that's the idea you meant to convey by making Diagonal a static class enclosed by the Piece class).

I've been listening to the audiobook of Blink by Malcolm Gladwell and I want to use a phrase from it: "a wave of intuitive revulsion."  That section of the book where that phrase is first mentioned perfectly describes what goes on when I get what's called "a sense of code smell." When that sense is strong, it does rise to the level of intuitive revulsion (the word that usually goes through my head is "YUCK!" in bold uppercase).

Now, what I felt when I saw Diagonal inside Piece was far from being at the level of revulsion but it definitely got my Spidey senses tingling. This is what I find fascinating. What do I sense that you didn't? Why do you think this is a good idea? Can I share my aversion to it and make you change your stance or will you double down and defend your decision?

Let me try to explain why I instantly found it smelly in my next reply.

 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
@Gerard: The immediate smell I got from scanning the code you showed is what I can only describe as "awkward semantics" which, now that I think about it, I closely associate with the smell of "misplaced responsibilities."

Would a Piece be so strongly associated with a Diagonal to the point of the code (almost) saying "a Diagonal has a strong dependency on a Piece"? I don't think so.

Diagonals, rows, and files are substructures of the Board, not the chess pieces themselves. Sure, the chess pieces, the Bishop with a Diagonal in particular, may interact with those substructures of the Board but I don't think their interaction means they are tightly coupled. That's kind of like saying my car has a very strong tie to the roundabout three miles down the road from my house.

Also, that a Piece would have intimate knowledge of the row and column it's on seems to me problematic. I say "intimate" because of the private row and column attributes you gave to Piece. If anything, I would have thought the knowledge would have been more abstract, like "a Piece knows its current Position on the Board" and "a Position represents a specific row and column on the Board".

Evidence of the smell of too intimate knowledge by Piece of the row and column it currently occupies is on lines 43-48 where validation of the values of row and column occurs. What business does a Piece have in validating values that relate to Position? I think none.



 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here's a small story I'd imagine telling with code:

With the output of:

c1 g5

I can then imagine implementation code that ends up looking like this:

I didn't come up with that code right off the top of my head, mind you. Before I submitted this reply, I went through several refactorings, massaging the code so that responsibilities were assigned properly. In the process, I tweaked that first section of high-level code to tell the story a little bit differently and the implementation code started out with a few lines of code that eventually was pared down to one line of code.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Here's another chunk of code that kind of put me off:

It's great that you have a unit test, which is more than I can say for 80% of developers. However, there are some problems that jump out at me when I read test code like this.

1. The System.out.println() is a test smell. It violates FIRST principles, in particular, S - self-verifying. Why do we need a display statement here? Are we unsure of the assertions? Is the test serving multiple purposes? Kent Beck says "a test should tell a story... it should have a moral, a point." My interpretation is that it should have a singular, coherent purpose.

2. The name createKingPawn only tells part of the story, particularly, the setup. I'll cite Kent Beck again: "A test should have an arc: there should be a beginning, a middle, and an end." The test name should give you a sense of what the arc of the story is. Also, "king pawn" is very specific. It makes me ask these questions: What, are we going to test all the pieces now? Should I expect a test for creating a Queen Pawn, a Queen Bishop Pawn, a Queen Knight Pawn, etc.? What's the point of all these tests? Is there a generalization we're trying to express here?

3. The assertions to equal 5 and 2. It seems the test is trying to imply that 'e' should be translated to 5 and '2' remains as 2. You have to read and analyze the test code and infer this. Above all, it smells of a leaky abstraction. This test speaks more about the underlying implementation once you analyze it thoroughly. This makes it a brittle test. Brittle tests are more costly in the long run and I try to avoid them if I can find a more intent-oriented way to write them, one that is relatively de-coupled from implementation detail.
 
Gerard Charles
Ranch Hand
Posts: 43
8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
All models are wrong. Some models are useful.

A bishop is only a bishop when it's on the board. Otherwise it's a funny shaped object. It works for me to model it's movement in the Piece / Bishop class hierarchy. If you prefer to put Piece motion in a Board, go for it.

The solution I've outlined addresses the question the original poster asked. The linked page https://open.kattis.com/problems/chess is a different problem and would have a different solution.

I'm a working engineer. Engineering is about maximizing the ratio benefit / cost . The purpose of a social media post is to convey an idea; what my post is trying to convey is how to efficiently model the diagonals on a chessboard. The test cases were sufficient for me to be reasonably confident the code is working as intended.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
So double down and defend then, huh? I'm a working engineer, too, by the way.

Edit: Coming back from Aikido class tonight, I was reminded once again that I must try to apply my training outside the dojo. My comments were not meant to put you down or call into question your competence or anything like that. See my remarks below about egoless programming. I am genuinely interested in examining other ways of looking at things but this doesn't preclude me making comments about the merits or lack thereof that I find when I read the code. I try to be as objective as possible even though some of these things can be fairly subjective. But the comments are purely about the code and design, not about the author, although I realize it can be difficult to dissociate oneself from the work you produced.

For what it's worth, I welcome any reasonable feedback/critique of code I've written so that I may have an opportunity to learn and improve.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:A bishop is only a bishop when it's on the board. Otherwise it's a funny shaped object. It works for me to model it's movement in the Piece / Bishop class hierarchy. If you prefer to put Piece motion in a Board, go for it.


As a working engineer who collaborates with other working engineers to create software, I find this response usually comes from people who don't quite understand that it's not about what they find makes sense. When I discuss the merits of code, or the lack thereof, I always frame it around what makes sense to everyone who reads the code. The mentality we have on our team is that it's "OUR" code, not any individual's code. So, it serves us all if we can go into some code and develop the same mental model from what the code says. It serves only the one person if only he/she can make sense of the code. So if "Go for it!" is the kind of attitude we get from any of the working engineers on our team, they will soon find themselves not working.
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
@Junilu: I know you do not like my interprétation BUT you will change your mind!! I actually made a short movie about fighting bishops, just wait for it 😎😎.

I just need first to get that wrapped in a custom React component, as soon as I have some free time....

@Gerard*: Thanks for your input! I do not have a modelling problem though, since I do not aim to deliver a software but solve a short puzzle on Kattis. It's not even supposed to be seen by human eyes ! The bug had been discovered and taken care of, and the C++ code solves in 0.01s.

I will still post a refactored solution on my blog, if ok with Junilu, or anyone who wants to donate good code!

*edit: I wrote Charles before, sorry about that!
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Many people get touchy and defensive when someone points out problems in code that they wrote. This is a natural reaction and its one of the hazards I have to navigate as a mentor to other developers. I always remind them about Egoless Programming and help them understand that the comments are made in that spirit.

@Gerard, the comments I made were not meant to criticize or question your skills or competence. They were meant to share and thus hopefully widen people's perspective on how code that made sense when it was written can easily have gaps in context that others cannot readily fill. Those gaps in context and resulting weirdness in semantics are often costly when taking the bigger picture of total cost of ownership into account. I was only sharing what I thought of the code and what ideas I was getting from reading it. The comments have nothing to do with you personally, but with how I process what I read from the code.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

D.J. Quavern wrote:@Junilu: I know you do not like my interprétation BUT you will change your mind!!


It's not about me not liking it. As you may have gathered, I'm all about people have a shared understanding of what it is we're talking about. Any misalignments in that understanding causes friction, bugs, frustration, and additional cognitive load. It's an unnecessary burden in an already complicated and difficult endeavor. So, the more people can speak in common terms and have a shared mental model, the easier it is to come to agreement on what to do and how to do it.

If the problem doesn't talk about Bishops murdering other Bishops, why would I agree to thinking about it that way? To be honest, it's kind of silly, I think.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
While I'm not much of a fan of the "Murdering Bishops" perspective, I don't hate it either. I just find it interesting and not particularly helpful for the exercise. My "objections" as you perceive them to be are simply because the idea of bishops murdering other bishops doesn't line up with the spirit of the problem statement. That's all. Since nobody is going to lose money or die from it, it's not really a big deal. As an exercise for learning, however, I'd rather do things properly or at least think in more appropriate terms.

I believe this is the last time I'll beat that dead horse though.
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Junilu Lacar wrote: Since nobody is going to lose money or die from it, it's not really a big deal.



Except the bishops you mean ?

Junilu Lacar wrote: I believe this is the last time I'll beat that dead horse though.



Wait till you see my movie!
 
Knute Snortum
Sheriff
Posts: 6266
167
Eclipse IDE Postgres Database VI Editor Chrome Java Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Gerard Charles wrote:Don't see how Diagonals could work with a single field, since each square is on two diagonals.


It turns out you don't need the Diagonals class at all!  just change the canAttack(Piece) to this:
Full disclosure: I didn't think of this on my own.  I ran into it while coding a "Queens attack" puzzle.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

D.J. Quavern wrote:Except the bishops you mean ?


LOL. It's nice to see you being such a good sport about all this. Assigning tone to text in a medium like this is a tricky business but you seem to be pitch perfect in this thread
 
D.J. Quavern
Ranch Foreman
Posts: 266
12
IntelliJ IDE Firefox Browser Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Knute Snortum wrote:

Gerard Charles wrote:Don't see how Diagonals could work with a single field, since each square is on two diagonals.


It turns out you don't need the Diagonals class at all!  just change the canAttack(Piece) to this:
Full disclosure: I didn't think of this on my own.  I ran into it while coding a "Queens attack" puzzle.



Yes, I have this test too. We are testing if the k value (as in y = k*x + m) are the same.

The bug I had though, was not the same diagonal, but when the bishop needed to make a U-turn to murd reach another position.
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Knute Snortum wrote:

Gerard Charles wrote:Don't see how Diagonals could work with a single field, since each square is on two diagonals.


It turns out you don't need the Diagonals class at all!  just change the canAttack(Piece) to this:


True. When I was playing around with this, I extracted that expression to get something like this:
 
Junilu Lacar
Marshal
Posts: 14039
234
Mac Android IntelliJ IDE Eclipse IDE Spring Debian Java Ubuntu Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Before I give it a rest, I just want to show how I would refactor the test code I critiqued earlier.

To tell the story that I think the above code is trying to tell better, I'd do something like this:

I think the above test code is clearer as to intent vs. what it was before and the tests are more focused and have singular purpose.
 
I didn't do it. You can't prove it. Nobody saw me. The sheep are lying! This tiny ad is my witness!
Java file APIs (DOC, XLS, PDF, and many more)
https://products.aspose.com/total/java
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!