Beta

Picross - generate clues

29BaliBalo
Description
Loading description...
Games
Algorithms
View
AllIssues3QuestionsSuggestions3Show Resolved
  • Please sign in or sign up to leave a comment.
  • dfhwze Avatar

    You should keep internal state of your class hidden to the user. My solution should not pass, I guess. Alternatively, don't use classes and just provide a string and expect an array as result.

  • Blind4Basics Avatar

    According to voile, the change about empty columns must be reverted (See thread below / sorry about that...)

  • Blind4Basics Avatar

    btw, I disagree with this part:

    By design, clues for empty rows and columns should be set as [0]rather than []

    a number in the array means "you have a segment of length x". So an empty array means there are no segment in the row/column. But [0] should actually mean there is a segment of length 0 in the row. Which doesn't really make sense...

    • BaliBalo Avatar

      I agree, with my "rather than" I was trying to imply "even if it is less semantic and intuitive than".

      "By design" in this case for me means "What has been decided for this project", which could be for example because most Picross games do show a 0 next to empty rows rather than leaving the clues empty (although I agree that this should rather be done when the clues get displayed rather than being actually internally represented by a 0). I added this as one of these maybe slightly suboptimal real-world type of constraints.

      I'm open to change it in the kata if there is a strong opinion against it, but in the end this is really just a convention, so I do not think this is that much of a problem as long as it is clearly specified.

    • Blind4Basics Avatar

      thing is, this 0 is related to an internal choice of your implementation. It feels like "I didn't find a simple way to get an empty array, so let's use [0] instead". That looks suboptimal and decreases (imo) the overall quality of the kata.

      To clarify, I don't see ideas like (summarizing roughly) "shits happen in real life, so let's implement some weird things here too" as good ideas. That's sort of reinforcing the idea that it's not a problem to create a bad design since it happens everywhere. I'm against this idea. A better train of thoughts is imo: "let's do our best. Maybe we'll make mistakes, but at least let's avoid antipatterns". See my point? So yeah, I don't like that [0] thing at all. ;)

    • BaliBalo Avatar

      Oh no my implementation (like most implementations actually I assume) would actually be simpler without this 0 constraint. I wrote the description and classes before writing a solution, so my implementation didn't have any impact on any of those. I just thought it would be almost equally likely in the development of a game like this to be decided to have a 0 (for any reason) rather than an empty array.

      Yeah I mean I agree it's not good to generalize bad designs, but I feel like katas are more to train coding itself than design decisions. And in this context, flexibility is a good skill to train.

      But yeah sure, I'll change it, I'm not really attached to that part of the challenge anyway

    • Voile Avatar

      Bruh, why is [0] changed to []? [0] is the standard representation of an empty row/column clue in Picross, so if we're doing a kata bout Picross we should be following standard notations.

    • Blind4Basics Avatar

      fine, but it still doesn't make any sense.

      Suggestion marked resolved by Blind4Basics 2 years ago
  • Blind4Basics Avatar

    looks like you messed something up about the height/width somwhere in your code (the non square test):

    width:4 height:2
board.column(0) [ Symbol(empty), Symbol(filled) ]
board.column(1) [ Symbol(filled), Symbol(empty) ]
board.column(2) undefined    -> with width of 4, why the third column is undefined??

    Note: you have to enforce some non square grids in the random tests too (I guess there aren't any since my code passes the random tests)

    • Blind4Basics Avatar

      here it is:

      isValidColumnNumber(index) { return typeof index === 'number' && index >= 0 && index < this.height; }
//     ^^^^^^                                                                               ^^^^^^
    • BaliBalo Avatar

      Thanks!! Apologies, I completely missed that one.

      And yeah the random tests are generating non-square grids, so I am unsure why they would work. I think my tests were generating wrong solutions which happened to match yours maybe? Not entirely sure

    • Blind4Basics Avatar

      I don't know... On my two solutions, the fist (long) one is the problematic one because I used board.column(...). You can explore its behavior with the random tests through its fork panel.

    • BaliBalo Avatar

      Oops, looks like I was testing my tests and forgot to re-enable them for the randomly generated ones... That explains it! I've addressed most of your comments btw (tried to make the description more explicit)

      Issue marked resolved by BaliBalo 5 years ago
  • Blind4Basics Avatar

    you should change the wording about the content of "cells": in the bullet points, you're talking all the time about "values", while you're using sort of "enums"/constante (EMPTY/FILLED) It's a bit confusing (mostly because your setup is somewhat overthought, imo)

    • BaliBalo Avatar

      Yeah I do think that "value" is still ok as for me it doesn't necessary imply any type. For example in my head even a function could be considered the "value" of a variable.

      But it does depends on everyone's definition of value and I agree that a portion of people would misinterpret it as being a number. I will try to clarify!

    • Blind4Basics Avatar

      About this, I feel like the only thing needed is reformatting a part of the description.

      Reomve this sentence:

      Each cell in the context of this kata will have a value of either PicrossBoard.FILLED or PicrossBoard.EMPTY.

      and push it as first bullet point just after, rewording it a bit:

      • Each cell has a value/state that can be either PicrossBoard.FILLED or PicrossBoard.EMPTY.

      With that, the info stands out more and that should resolve the ambiguity.

    • BaliBalo Avatar

      Thanks! I will switch to that

  • Blind4Basics Avatar

    Hi,

    It's not clear what the result should be: is the PicrossClues class already implemented? (looking a the initial solution, it seems so => just say it in the description).

    Why a class while a simple object {rows:...,columns:...} could have been enough?

    • BaliBalo Avatar

      Hi! Yes sorry, I tried to also indicate it through the initial solution but I understand that this is likely not clear enough, I will try to make it clearer.

      The class was to try to be closer to a real-world project, in which people around you (your project's management team, your coworkers, ...) might take decisions like this. This could be justified by saying that for example to make a playable game later it helps to have some available functions (I didn't add any in this case but you could imagine for example adding prototype functions to automatically keep track of which rows / columns have been filled and show that to the user) - and some people / businesses would prefer to have it that way rather than a collection of independant functions to which you would need to always pass the clues object.