Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intellij complains about dependencies in ConsoleTest.kt. #548

Open
pcholt opened this issue Feb 1, 2022 · 20 comments
Open

Intellij complains about dependencies in ConsoleTest.kt. #548

pcholt opened this issue Feb 1, 2022 · 20 comments

Comments

@pcholt
Copy link
Contributor

pcholt commented Feb 1, 2022

@pcholt Intellij complains about your dependencies in your ConsoleTest.kt. I had to disable the kotlin plugin for it to shutup.

Originally posted by @jnellis in #541 (comment)

@jnellis
Copy link
Contributor

jnellis commented Feb 1, 2022

The issue is that the IDE defaults to trying to compile kotlin source files. But it has no guidance because there are no project specific directions (which it shouldn't because you'd be dictating your project preferences on everyone.) If you don't have the kotlin plugin turned on it will ask you everytime if you want to turn it on because it detects kotlin files. So the scenario is that I'm a newbie and have my IDE installed but then I get this popup saying turn on kotlin, so I do, then I get all these dependency errors because there is no project files saying where these dependencies are located (maybe this assumes you imported as a gradle project for the runGames directory.) Now I'm discouraged because I don't know what's going on and decide to not pursue further.

Maybe put this class as an inner class in a gradle build file if that's the point. That will have a .kts extension and since if you didn't import the project as gradle, the ide will ignore it.

@pcholt
Copy link
Contributor Author

pcholt commented Feb 2, 2022

Still a little confused. You say there are dependency errors? Which project are you building to get dependency errors?

There's a dependency on the ConsoleTest.kt library in any kotlin project that includes tests. I could solve this by

  • compiling and hosting consoleTest.jar on a repository somewhere, or
  • adding a flag in gradle.properties to stop the dependency on consoleTest when there are no tests, or when there are no tests which use it.

As long as test projects depend on the :00_build_utilities project, they will need a way of compiling kotlin to build the test library.

@jnellis
Copy link
Contributor

jnellis commented Feb 3, 2022

Your gradle build is leaking into the rest of the project is what I'm saying.

import com.google.common.truth.Truth
import org.junit.Rule

This is only declared as a dependency in your gradle build. Intellij defaults to compiling EVERY file it knows about if there is not a project file. But I'm not using gradle, I chose NOT to import this project as a gradle project for the shear security problems it offers for future shenanigans. So now when I try to compile a java file, Intellij want's to compile all your kotlin test files but it can't because it doesn't know where the imports to those third party dependencies come from, and thus throws up unrelated compiler errors to whatever I'm working on. This is what people that choose to not import the project as a gradle build will see.

@coding-horror
Copy link
Owner

So am I clear to delete this folder?

https://github.com/coding-horror/basic-computer-games/tree/main/buildJvm

@jnellis
Copy link
Contributor

jnellis commented Mar 25, 2022

I would but that's just my opinion. This folder is the essence of scope creep. It's a gradle build that contains its own wrapper, which is basically a convenience kitchen sink of essential gradle files to allow you to build a project without installing gradle. All it does is compile to executables for convenience. For that matter any IDE specific or build specific files should be left out of any languages because they are able to touch the system that people run these on, usually via their IDE, and usually automatically done like in the case of Intellij. So unless you are a gradle expert, you can't really approve any changes to the build file (or the 100 individual build files in subfolders) unless you know they won't rm -rf peoples harddrives. I don't do C# but I see visual studio project files and some semblance of C# 'utilities' evolving in the 00 directories. Python too.

So you didn't want enterpriseyness to creep in but that's what has happened. He has added test files outside of this buildjvm folder into some of the numbered game folders, all of which inherit from a base class in the utilities folder, which itself has third party dependencies that can only be resolved IF you accept the gradle project in the buildjvm.

I really don't think this project needs any dedicated testing folders, testing frameworks, testing helper classes, IDE specific project files of any kind, third party dependencies, or build systems. Anything that you can't use the interpreter/compiler to compile/run the ONE file that is a port of the corresponding BASIC game. Otherwise it becomes too complicated for beginners to decipher, and too much to maintain, especially from a security standpoint. Not to mention the bloat in the repository, including the history as someone has just pointed out.

So to summarize, if you delete this buildjvm folder, you have to delete the kotlin test folder in 00Utilities, and the individual tests that rely on that kotlin test folder, which are scattered about the 100 folders. I don't think there are many because they rely on games that are deterministic and don't have any random() usage I believe.

@coding-horror
Copy link
Owner

coding-horror commented Mar 25, 2022

For that matter any IDE specific or build specific files should be left out of any languages because they are able to touch the system that people run these on, usually via their IDE, and usually automatically done like in the case of Intellij. So unless you are a gradle expert, you can't really approve any changes to the build file (or the 100 individual build files in subfolders) unless you know they won't rm -rf peoples harddrives. I don't do C# but I see visual studio project files and some semblance of C# 'utilities' evolving in the 00 directories. Python too.

I see, so we are considering a policy of "no IDE specific or build specific files please" for the project? I am tentatively in favor of that since it keeps the repository clean.

However, I do think a 00_Utilities folder is useful, provided those don't bleed into the individual game folders?

what do you think @MartinThoma and @mojoaxel ?

@MartinThoma
Copy link
Collaborator

I see, so we are considering a policy of "no IDE specific or build specific files please" for the project? I am tentatively in favor of that since it keeps the repository clean.

In general I like that policy, but there are a few exceptions I would make, eg for project-wide linting/formatting configuration or the. gitignore file.

I do think a 00_Utilities folder is useful, provided those don't bleed into the individual game folders

What do you mean with that? What is "those"?

I was wondering about this folder actually. In the python projects I see very few shared functions (eg one for printing tabs). Is the idea to keep them in one place instead of having them in every game?

Personally, I would prefer to isolate all game code completely. This is way more beginner friendly.

@coding-horror
Copy link
Owner

coding-horror commented Mar 25, 2022

Personally, I would prefer to isolate all game code completely. This is way more beginner friendly.

We could also make that a policy.. but I tend to prefer having a handful of shared libraries for common things like console input etc, just so that every game isn't duplicating really silly, simple things like "am I accepting uppercase and lowercase" and "am I properly responding to non-numeric input when I said I wanted a number" etc? I guess we'd need to look at an example. Pick one game, and see how many lines of code are saved by having just a few shared common functions?

@jnellis
Copy link
Contributor

jnellis commented Mar 26, 2022

just so that every game isn't duplicating really silly, simple things

I like the idea of snippets over actually importable helper or utility files. I admittedly at first tried to tackle an importable utility file for user input but as I did a few games it was clear there's really not a single good way to input a string, or a number or a comma separated list or a string and a number without forcing the programmer to also create some mangling glue code around that utility function that is specific to the problem. The end result being more code than what is really necessary. That's why a non-compilable readme of snippets seems like a better approach. You copy/paste what you need then alter it from there. The other issue with a utility folder is that its in a separate folder. Most of these BASIC programs are like 150 lines of code, how did modern languages get so 'modern' that we need to dip outside the folder for help. The ports of these BASIC programs should be lengthwise similar without tricks. Sadly, user input was already a rough edge in BASIC and cleanly doing it in a port is simply going to require a higher line count. The Mastermind program is a good example, it wants number input, it want's a specific letter input(denoting colors), it want's number separated by comma input, and finally it wants some keyword input like BOARD, and QUIT. I wrote a generic method for that that takes a prompt, a closure for the extracting the input type, and a predicate to validate and the amount of code for user input still is more than the generic part simply because mangling text to what you need is messy in Java. Your untyped languages are going to be easier to deal with user input imo and less need for a utility.

Here's an example. The TAB method requires terminal column output location which no languages maintain and so any kind of verbatim output attempts require line buffering which might not be very fluid to ones style. Most submissions I see ignore TAB mechanics for something reasonably similar. A utility method that people feel obliged to shoehorn into their code forces their code to be structured a certain way.

I like readme's that break down the BASIC programs. Some of them are truly a spaghetti nightmare. I noticed recently in a pull request for the Mastermind game that there is an error in the BASIC code that disallows the computer to guess correctly. I did notice something funky about that program but I'm not sure of the conclusion that was come to about the logic being fully wrong. I wrote a java port but haven't submitted it yet and I ignored the BASIC programs logic and wrote my own based on the intent of the solution because of the funkiness of what I saw. A few months ago, about half of the AceyDucey submissions regarding random card picking were wrong. There are plenty of bugs in the originals that would be a job in itself just unwinding into a 'porting guide.'

So those two things really, snippets and analysis are what should be in utility folder. The javascript folks have some leeway as their stuff is directly implementable and I like the idea of linking to a github pages directory. But the other languages require an interpreter/compiler, on the user machine mostly, and so any executable mumbo jumbo outside of the program they are working on is questionable. Making editor config or linter directive files available is cool but I don't think that a style should be enforced beyond the language's conventional style (e.g. Capitalized class names) Things like tab vs spaces, line column limits (within reason), bracket placement, or how continuing lines or function parameters line up or not, should not be enforced. If you can't read it, ask to clean it up but style is a personal preference and you can always learn something from someone else's style in my opinion.

@MartinThoma
Copy link
Collaborator

MartinThoma commented Mar 26, 2022

Looking at the Python implementations, I could see variations of the following functions in a couple of games:

def print_n_whitespaces(n: int) -> None:
    print(" " * n, end="")


def print_n_newlines(n: int) -> None:
    print("\n" * max(0, n-1))


def print_with_tab(space_count: int, msg: str) -> None:
    print(" " * space_count + msg)

Importing this is a bit of a hack as the project overall does not have a Python package structure. Hence one needs to manipulate the Python PATH:

import sys
from pathlib import Path

# Expand PYTHONPATH in order to be able to import
# Make sure it's relative to the current file, not to the current shells path
utils_folder = Path(__file__).parent / "../../00_Utilities/"
sys.path.insert(0, str(utils_folder))

# Now you can import
import markdown_todo

As all potentially shared functions I've seen are trivial 1-2 liners (where I'm uncertain if they should have their own function in the first place), I'd rather not use shared functionality for the Python implementations.

edit: I forgot that there is also the functions that deal with user input, e.g. searching for def .*prompt.*:

def ask_bool(prompt: str) -> bool:
    while True:
        answer = input(prompt)
        if answer == "YES":
            return True
        elif answer == "NO":
            return False
        else:
            print("INCORRECT ANSWER - - PLEASE TYPE 'YES' OR 'NO'.")
            
def get_number_from_user_input(prompt: str, min_value: int, max_value: int) -> int:
    """gets a int integer from user input"""
    # input loop
    user_input = None
    while user_input is None or user_input < min_value or user_input > max_value:
        raw_input = input(prompt + f" ({min_value}-{max_value})? ")

        try:
            user_input = int(raw_input)
            if user_input < min_value or user_input > max_value:
                print("Invalid input, please try again")
        except ValueError:
            print("Invalid input, please try again")
    return user_input
 

def prompt_too_much_or_too_little():
    answer = input().upper()
    if answer == "TOO MUCH":
        return True, True
    elif answer == "TOO LITTLE":
        return True, False
    return False, None

Although they are all similar, I'm uncertain how easy it would be to generalize those.

@coding-horror
Copy link
Owner

What do you think @mojoaxel ?

@mojoaxel
Copy link
Collaborator

So, I took some time to think:

I think we should have a look at the Project goals. I think we could sum them up with just two:

  1. Let people see how games looked like 50 Years ago. I think we reached this goal by making the games playable online.
  2. Provide fun little beginner scripts so that people can have a look into different programming languages and play around with them. For that it is important, that the code is very easy to read (well structured and well documented) and the scripts are easy to execute. Here we can invest more!

I thing anything else than this should maybe done in a different project or a fork.

We should concentrate on simple "scripts" that have as little external dependencies as possible (none). I would guess that all supported languages provide simple CLI Input/Output and random numbers without external packages (but I might be wrong here). Therefor I would recommend to stick to a one game one file-rule.

I honestly would recommend to not have "COMMON" folder at all. I think the basic IO could easily be done inside each script. For us experienced developers this might feel very wrong because we will see a lot of duplicated code, but the benefit is, that scripts are very easy to execute and can easily be copied around, posted (e.g. in magazins 😉 ) and just "live" on there own, like the originals BASIC scripts did. I very much like this idea!
One downside is that every game need to have its own (short) implementation of a PRINT an INPUT and a TAB function, but is this realy a problem? It might even be an advantage to see different ways to implement something like TAB 😃
Another downside is the handling of input. If we do not have common IO-functions some games may allow case-insensitive inputs other may not, but I think that is not realy a problem and we could even embrace it!

So, after looking into it I would recommend removing the following files and folders: ☢️

00_Common/BASIC_Tests
Sorry but this is just clutter that might confuse beginners!

00_Common/dotnet
Creating very complex interfaces for something simple as IO is not needed in my opinion. This looks more like a professional approach, that is simply not worth doing for simple 20-liner games. Also this folder is very Visual Studio (windows) specific and simply not needed!

.NET project files
I see why e.g. in C#/VB IDE files (e.g. .csprojand .sln ) could be useful, but they are simply not needed and clutter the project. I for example run these C# programs on linux on the command line using mono and do not need these files. I think we should get rid of them. User easily could just copy the content into there preferred IDE.
(We could even discuss if the separation into multiple class files in C# is realy useful!)

"test" files
I have no idea why we would need test files in this project. Testing obviously is always a good thing but here it is very confusing to me!. We should "test" these simple games just by playing them and having fun 🎮 . I think we should remove all these test_*.py files from the python folders!

00_Utilities/jvmTestUtils/kotlin/test
The same here. I don't think "test" are needed and complicate things!

00_Utilities/DotnetUtils
To be honest I have no idea what this folder is for and if it is needed. I don't think so!

I voices strong an probably controversial opinion here and I hope you don't mind! I realize that the proposed "purge" would throw away a lot of code from other people and that might hurt, but I was ask for an opinion, well that was it!

@MartinThoma
Copy link
Collaborator

I like keeping files easy to execute 👍 I also like tests when I refactor stuff 😄 They actually saved me some times because I do not always play the game when I re-write stuff 😅 We do have a couple of Python scripts that are WAY more complicated than they need to be, but simplifying them is also not always trivial as they sometimes do super weird stuff. I don't want to blame the original devs; it's just hard to go from the old code to something modern.

I'm wondering if it's always possible to keep a very strict 1-file rule. I see three questions here:

  1. Is having 1 file only (too) bad style? For some languages (like Python) I don't think it's too bad. But I think the Java-people feel more strongly about having multiple classes in one file. Also for JavaScript + HTML I could imagine that things become actually harder to read/understand; even for beginners. For C# I see .csproj / .sln / .cs files ... I have no clue if those could be just one file.
  2. Do we want to have alternative implementations? In 01_Acey_Ducey/python we have 3 different implementations. Do we want that?
  3. Do we want to allow files for "game-local" configuration? e.g. 01_Acey_Ducey/javascript/.prettierrc.json. This way a contributor would not need to think about anything execpt what is in that folder.

For the "little beginner scripts" goal: I liked to have some tests in here and thought it would help people who have already written some scripts, but start to grow from there. When I started software development, it took me quite long to learn how to test code. I had only seen super complex test setups or completely trivial ones. Especially how to deal with user input / randomness / external services took me a while. I would like to share it, but it might be that this repository is not the best way.

@coding-horror
Copy link
Owner

coding-horror commented Mar 26, 2022

So there are a few things being discussed

  1. Don't allow large files in repo, this one is no-brainer, we should definitely do it ✅

  2. Don't allow ANY IDE or build environment specific files, I think we should definitely do this ✅

  3. Don't allow ANY shared code between programs ⁉️

Ok this last one number 3, I am having trouble with. I get the idea of having completely standalone files, but I don't like the idea of every single program re-implementing basic error handling (oh, you entered "foo" but I expected a number), and we did say we want the programs to be modernized to accept uppercase/lowercase and be more tolerant of errors rather than just crashing or whatever. I also feel like "why not do some unit testing?" is a good thing to teach developers, even beginning developers!

I think our rule should be

  1. Keep to a minimum of shared code between programs, only the most essential and repeated functions (we should provide a specific example).

@mojoaxel
Copy link
Collaborator

mojoaxel commented Mar 26, 2022

I don't like the idea of every single program re-implementing basic error handling (oh, you entered "foo" but I expected a number), and we did say we want the programs to be modernized to accept uppercase/lowercase and be more tolerate.

I can live with that! But than we should keep it simple and just implement basic funktions for PRINT, INPUT, TAB to not over-complicate things, like it has e.g been done for .NET.

I also feel like "why not do some unit testing" is a good thing to teach developers.

What is the "unit" in a small script that just consists of a loop and some conditions?

Could we at least move test files to some "tests" folder? I'm not sure that beginners will get the meaning of test_ files immediately.

Also be aware: test also often mean complicated testing frameworks or at least external packages that need to be Installed.

@coding-horror
Copy link
Owner

Yeah true. I have mixed feelings on introducing testing to these very simple programs. Maybe we should have a rule that

  • a handful of common shared functions

is more practical to beginners than

  • learning testing conventions from scratch

And as far as "handful of common shared functions" we should lead by example.. pick 3-4 programs, extract just the commonest stuff, and point to that as an example of what we mean?

We don't want framework-itis, just .. less code repetition.

@MartinThoma
Copy link
Collaborator

Just to be sure that I understand you @coding-horror : This means I should remove the Python unit tests (+ not allow PRs to add new ones), right?

@MartinThoma
Copy link
Collaborator

I'm not sure why

curl -s https://api.github.com/repos/coding-horror/basic-computer-games | grep size | tr -dc '[:digit:]'

still shows 75310 (75 MB). I guess the reason could be the branch https://github.com/coding-horror/basic-computer-games/tree/74-minimal-terminal

@coding-horror
Copy link
Owner

coding-horror commented Mar 28, 2022

I have added the following rules to the readme.md

⚠️ Please note that we have decided, as a project, that we do not want any IDE-specific or build-specific files in the repository. Please refrain from committing any files to the repository that only exist to work with a specific IDE or a specific build system.

and

  • Please don't check in any build specific or IDE specific files. We want the repository to be simple and clean, so we have ruled out including any IDE or build system specific files from the repository. Git related files are OK, as we are using Git and this is GitHub. 😉

I also made a note about "no enterprise coding practices", e.g. the underlined words below 👇

Please don’t get too fancy. Definitely use the most recent versions and features of the target language, but also try to keep the code samples simple and explainable – the goal is to teach programming in the target language, not necessarily demonstrate the cleverest one-line tricks, or big system "enterprise" coding techniques designed for thousands of lines of code.

cc @MartinThoma and @mojoaxel 👆

@coding-horror
Copy link
Owner

still shows 75310 (75 MB). I guess the reason could be the branch https://github.com/coding-horror/basic-computer-games/tree/74-minimal-terminal

Should we delete this branch? I don't even know what it is?

(and yes, I think we should leave unit testing alone now in favor of some very light shared code for common functions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants