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

Refactoring #44

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Refactoring #44

wants to merge 29 commits into from

Conversation

Ahzed11
Copy link
Collaborator

@Ahzed11 Ahzed11 commented Oct 24, 2021

Refactoring

@Ahzed11 Ahzed11 self-assigned this Oct 24, 2021
@Ahzed11
Copy link
Collaborator Author

Ahzed11 commented Oct 24, 2021

File structure

  • Validate the structure
  • Some exercises like ComparatorAncCollection use two different classes, do I put them together in main?

Files

  • Some exercises have code blocks in their descriptions. Do I keep the old synthax (From Inginious) or should I adapt it?

@AlexandreDubray
Copy link
Member

Some comments

  • Yes if the exercise use two classes you should put them in the same file
  • All the inginious-test related packages/annotations should be removed and generated at the same time as the inginious course. A typical example of test file should be
import static org.junit.Assert.assertEquals

public class ExerciseTest {

    @Test 
    public void test1() {
         // some tests that call the student's code
    }
}

Also don't forget to add the //BEGIN STRIP and //END STRIP so the students have a small test to launch when downloading the intellij project

  • The test files should be renamed {Exercice}Test.java
  • You must not make so many directories, the structure should looks like this
src/
|--- main/
      |--- java/
           |--- module1/
                |--- AsciiDecoder.java
                |--- Anagram.java
                ...
|--- test/
      |--- java/
           |--- module1/
                |--- AsciiDecoderTests.java
                |--- AnagramTests.java
                ...

And then starting from this the python script generates an inginious directory (but it should not be present at the beginning).

As a start, do not handle inginious things. Just make a repo with the exercises and the structure shown above. We should be able to run a command like mvn test and all the tests should ran and pass.

@Ahzed11
Copy link
Collaborator Author

Ahzed11 commented Oct 25, 2021

Sorry for bothering you but do you know a way to make IntelliJ understand that the files in src/ are Java classes ? I can't get any autocompletion when working in that folder.

@AlexandreDubray
Copy link
Member

Looks better for the structure 👍

For intellij you need to tell it that it is a java code repository, you can for example add a pom.xml at the root and use maven to run it.

@Ahzed11
Copy link
Collaborator Author

Ahzed11 commented Nov 4, 2021

When working on section 3 I encountered a problem with auxiliary classes.

The three exercises about Binary Trees include a Node class with the same name which means we have three different definitions of the same class in the same package.

Should I rename the Node classes or will we use a different solution to this problem ?

@AlexandreDubray
Copy link
Member

You can define the node class as a private class in the Tree classes. Something like

public class Tree {
    private class Node {
        ....
    }
}

should work. You can define getters to access elements of the nodes.

Also could you
i) Not push .iml files that are specific to intellij?
ii) Put the src node at the root of the project. The idea is that the repo is a java project
iii) You can delete the old folders as you go on. What's need to be done will be easier to see

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

Successfully merging this pull request may close these issues.

2 participants